feat: Update global api

[NeaguGeorgiana23]

This PR

• Adds getter and setter methods for Hooks in the global API.
• Updates the Shutdown method to also clean hooks when it is called.
• Adds unit test for these new getters and setters.
• Adds unit tests for SetEvaluationContext and GetEvaluationContext since they were missing.

Fixes #122

[coderabbitai]

Warning

Review limit reached

@NeaguGeorgiana23, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 48 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 77fae798-b692-4f63-bb10-b4e0ac0c74d4

📥 Commits

Reviewing files that changed from the base of the PR and between 04d9099 and 3c74f5d.

📒 Files selected for processing (1)

• openfeature/openfeature_api.h

📝 Walkthrough

Walkthrough

Adds typed C++ hook interfaces and global hook management to OpenFeature. Provider queries gain explicit default-provider overloads. Hook registration uses synchronized storage and clears during shutdown. Tests cover hook dispatch, registration, contexts, provider status, null filtering, and cleanup.

Changes

Hook infrastructure

Layer / File(s)	Summary
Typed hook contract openfeature/hook.h, openfeature/flag_evaluation_details.h	Adds Hook<T> lifecycle callbacks and common hook aliases. Updates include-guard comments.
Global hook management openfeature/openfeature.h, openfeature/openfeature_api.h, openfeature/openfeature_api.cpp, openfeature/BUILD	Adds explicit default-provider overloads and synchronized GeneralHook registration, retrieval, and shutdown cleanup.
Hook API validation test/BUILD, test/hook_test.cpp, test/openfeature_api_test.cpp	Adds hook dependencies and tests for typed dispatch, registration order, null filtering, contexts, provider statuses, and shutdown cleanup.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant OpenFeatureAPI
  participant HookStorage
  Client->>OpenFeatureAPI: AddHook or AddHooks
  OpenFeatureAPI->>HookStorage: append non-null GeneralHook instances
  Client->>OpenFeatureAPI: GetHooks
  OpenFeatureAPI->>HookStorage: read hook snapshot under shared lock
  HookStorage-->>OpenFeatureAPI: return registered hooks
  OpenFeatureAPI-->>Client: return hook list
  Client->>OpenFeatureAPI: Shutdown
  OpenFeatureAPI->>HookStorage: clear registered hooks under exclusive lock

Loading

Possibly related PRs

• open-feature/cpp-sdk#112: Provides the hook foundation used by this change.
• open-feature/cpp-sdk#121: Adds related provider-level hook support using GeneralHook.
• open-feature/cpp-sdk#127: Modifies the same global hook APIs and lifecycle behavior.

Suggested reviewers: oxddr

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 35.71% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and related to the global API changes, although it does not name hook management.
Description check	✅ Passed	The description clearly covers hook management, shutdown cleanup, and evaluation-context tests included in the changeset.
Linked Issues check	✅ Passed	The implementation adds global hook registration and retrieval, clears hooks on shutdown, and tests these behaviors, satisfying [#122].
Out of Scope Changes check	✅ Passed	The substantive changes support global API hook management and the explicitly stated evaluation-context test objectives.

✨ 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: 3

Caution

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

⚠️ Outside diff range comments (1)

test/openfeature_api_test.cpp (1)

19-25: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Ensure test isolation matches the comment.

The comment indicates that the singleton is shut down before and after each test to reset its state. However, SetUp() is currently empty. To prevent potential state leakage from other test suites that might run beforehand in the same process, consider applying the cleanup logic in SetUp() as well.

🛠️ Proposed fix

   // To ensure test isolation for the singleton, we shut it down before and
   // after each test, to reset it to its default state.
-  void SetUp() override {}
+  void SetUp() override {
+    api.Shutdown();
+    api.SetEvaluationContext(EvaluationContext::Builder().build());
+  }
   void TearDown() override {
     api.Shutdown();
     api.SetEvaluationContext(EvaluationContext::Builder().build());
   }

🤖 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 `@test/openfeature_api_test.cpp` around lines 19 - 25, Update the test
fixture’s SetUp method to perform the same singleton cleanup as TearDown: call
api.Shutdown() and reset the evaluation context with a newly built default
EvaluationContext. Keep the existing TearDown cleanup unchanged so isolation
occurs both before and after each test.

🤖 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-5: Add a direct <utility> include in flag_evaluation_details.cpp
alongside the existing standard-library includes so the translation unit
explicitly provides std::move without relying on transitive includes.

In `@openfeature/openfeature_api.cpp`:
- Around line 80-84: Update OpenFeatureAPI::AddHooks to skip null shared
pointers before inserting elements into hooks_, matching the validation
performed by AddHook. Preserve moving valid hooks into the collection while
ensuring nullptr entries are never stored.

In `@openfeature/provider.h`:
- Line 28: Update FeatureProvider::GetHooks() to provide a default empty
implementation instead of remaining pure virtual, preserving source
compatibility for downstream providers while allowing providers to override it
when needed.

---

Outside diff comments:
In `@test/openfeature_api_test.cpp`:
- Around line 19-25: Update the test fixture’s SetUp method to perform the same
singleton cleanup as TearDown: call api.Shutdown() and reset the evaluation
context with a newly built default EvaluationContext. Keep the existing TearDown
cleanup unchanged so isolation occurs both before and after each test.

🪄 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: a865e274-ec3d-4840-9f04-56e938238aab

📥 Commits

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

📒 Files selected for processing (22)

• 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/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/openfeature.h
• openfeature/openfeature_api.cpp
• openfeature/openfeature_api.h
• openfeature/provider.h
• test/BUILD
• test/evaluation_options_test.cpp
• test/flag_evaluation_details_test.cpp
• test/hook_test.cpp
• test/mocks/mock_feature_provider.h
• test/openfeature_api_test.cpp

[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/openfeature_api.cpp`:
- Around line 88-95: Update OpenFeatureAPI::AddHooks to acquire hooks_mutex_
with a std::unique_lock before reserving or appending to hooks_, matching the
synchronization used by AddHook; keep the existing null filtering and move
behavior unchanged.

🪄 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: a729b14d-7df5-4c6e-aedb-9f2fd1dbdc86

📥 Commits

Reviewing files that changed from the base of the PR and between ea6a463 and 665914a.

📒 Files selected for processing (4)

• openfeature/openfeature.h
• openfeature/openfeature_api.cpp
• openfeature/openfeature_api.h
• test/openfeature_api_test.cpp

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

• openfeature/openfeature.h

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

openfeature/openfeature_api.cpp (1)

109-113: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Release the mutex before destroying hook objects.

If hooks_ owns the last std::shared_ptr reference, hooks_.clear() runs the hook destructor or custom deleter while hooks_mutex_ is locked. If that code calls GetHooks(), AddHook(), or Shutdown(), the non-recursive mutex deadlocks. Swap the vector under the lock, then destroy the old vector after unlocking.

Proposed fix

 void OpenFeatureAPI::Shutdown() {
   provider_repository_.Shutdown();
-  std::unique_lock lock(hooks_mutex_);
-  hooks_.clear();
+  std::vector<std::shared_ptr<GeneralHook>> hooks_to_release;
+  {
+    std::unique_lock lock(hooks_mutex_);
+    hooks_to_release.swap(hooks_);
+  }
 }

🤖 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/openfeature_api.cpp` around lines 109 - 113, Update
OpenFeatureAPI::Shutdown so hooks_ is moved or swapped into a local vector while
hooks_mutex_ is held, then release the lock before the local vector is
destroyed. Preserve provider_repository_.Shutdown() and ensure hook destructors
or deleters cannot run while hooks_mutex_ is locked.

🧹 Nitpick comments (1)

test/hook_test.cpp (1)

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

Cover all supported flag types in this test.

GeneralHookReceivesAnyFlagType only exercises bool. Add cases for std::string, int64_t, double, and object values. This verifies each FlagEvaluationDetails<T> conversion through GeneralFlagEvaluationDetails.

🤖 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 `@test/hook_test.cpp` around lines 126 - 143, The
GeneralHookReceivesAnyFlagType test currently covers only boolean values; extend
it with cases for std::string, int64_t, double, and object values. For each
case, construct the corresponding flag context and FlagEvaluationDetails, invoke
GeneralTrackingHook::Before and After, and assert the converted default and
evaluated values through GeneralFlagEvaluationDetails.

🤖 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.

Outside diff comments:
In `@openfeature/openfeature_api.cpp`:
- Around line 109-113: Update OpenFeatureAPI::Shutdown so hooks_ is moved or
swapped into a local vector while hooks_mutex_ is held, then release the lock
before the local vector is destroyed. Preserve provider_repository_.Shutdown()
and ensure hook destructors or deleters cannot run while hooks_mutex_ is locked.

---

Nitpick comments:
In `@test/hook_test.cpp`:
- Around line 126-143: The GeneralHookReceivesAnyFlagType test currently covers
only boolean values; extend it with cases for std::string, int64_t, double, and
object values. For each case, construct the corresponding flag context and
FlagEvaluationDetails, invoke GeneralTrackingHook::Before and After, and assert
the converted default and evaluated values through GeneralFlagEvaluationDetails.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9bbb8101-6667-4502-85f8-0bb3e57eb28d

📥 Commits

Reviewing files that changed from the base of the PR and between 665914a and 04d9099.

📒 Files selected for processing (9)

• openfeature/BUILD
• openfeature/flag_evaluation_details.h
• openfeature/hook.h
• openfeature/openfeature.h
• openfeature/openfeature_api.cpp
• openfeature/openfeature_api.h
• test/BUILD
• test/hook_test.cpp
• test/openfeature_api_test.cpp

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

• openfeature/hook.h
• openfeature/openfeature.h
• test/openfeature_api_test.cpp