Provider hook evaluation order

[jonathannorris]

Addresses multi-provider gaps outlined in #1882 by implementing event aggregation, per-provider hook execution, tracking forwarding, and a new comparison strategy.

---

  

[cursor]

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
_{Learn more about Cursor Agents}

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly refines the MultiProvider functionality to address complexities associated with managing multiple feature providers. It introduces robust mechanisms for event aggregation, ensuring that the overall state of the MultiProvider accurately reflects its child providers. Furthermore, it enables per-provider hook execution, allowing for isolated and context-aware hook processing, and implements a new comparison strategy for evaluating flags across multiple providers. These changes improve the stability, predictability, and extensibility of multi-provider setups.

Highlights

• Event Observer Mechanism: Implemented a new event observer mechanism in EventProvider to allow external components to react to provider events, enhancing event handling flexibility.
• Comparison Strategy: Introduced a ComparisonStrategy for MultiProvider to evaluate multiple providers concurrently and compare their results, falling back to a designated provider if results differ or errors occur.
• Enhanced MultiProvider Functionality: Refactored MultiProvider to handle unique naming for child providers, aggregate their states, forward events, and execute hooks on a per-provider basis, ensuring isolation and proper context.
• Comprehensive Testing: Added extensive tests for the new comparison strategy, event aggregation, tracking forwarding, and per-provider hook execution to ensure robustness and correctness.

Changelog

• src/main/java/dev/openfeature/sdk/EventProvider.java
  • Added eventObservers list using CopyOnWriteArrayList for thread-safe event listener management.
  • Introduced addEventObserver and removeEventObserver methods to manage event listeners.
  • Modified the emit method to notify registered eventObservers in addition to the existing eventProviderListener and onEmit callback.
• src/main/java/dev/openfeature/sdk/multiprovider/ComparisonStrategy.java
  • Added new class ComparisonStrategy implementing the Strategy interface.
  • Provided a mechanism to evaluate all configured providers concurrently for a given flag.
  • Implemented logic to compare successful evaluation results and return the result from a specified fallbackProvider if all agree.
  • Added an optional onMismatch callback if provider evaluations disagree.
  • Included error handling and aggregation for individual provider failures.
• src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java
  • Added imports for various SDK components like ClientMetadata, Hook, ProviderEvent, ProviderState, etc.
  • Introduced fields to manage providerStates, providerEventObservers, hookClientMetadata, emptyHookHints, and aggregateState.
  • Modified buildProviders to ensure unique provider names by appending suffixes (e.g., "provider-1") if duplicates exist.
  • Implemented initializeProviderStates to set initial states for all child providers.
  • Updated the initialize method to register child provider observers, handle provider initialization errors, and update provider states.
  • Overhauled flag evaluation methods (getBooleanEvaluation, getStringEvaluation, etc.) to wrap individual provider evaluations with evaluateWithProviderHooks, enabling per-provider hook execution.
  • Added track method to forward tracking events to child providers, skipping those in NOT_READY or FATAL states.
  • Enhanced shutdown to unregister child provider observers and reset aggregate state.
  • Introduced several private helper methods for managing provider events, state aggregation, error handling, tracking, context handling, exception conversion, and hook context creation.
  • Defined an inner static class HookExecution to encapsulate hook and hook data.
• src/test/java/dev/openfeature/sdk/multiprovider/ComparisonStrategyTest.java
  • Added new test file containing unit tests for the ComparisonStrategy.
  • Tested scenarios where providers agree, disagree (triggering mismatch callback), and when providers fail.
  • Verified correct fallback behavior and error handling.
• src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderEventsAndTrackingTest.java
  • Added new test file for MultiProvider's event aggregation and tracking forwarding.
  • Included tests for aggregating child provider states (READY, STALE, ERROR, FATAL) and forwarding configuration change events.
  • Verified that tracking events are forwarded to active providers and skipped for fatal ones.
• src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderHooksTest.java
  • Added new test file demonstrating per-provider hook execution within MultiProvider.
  • Tested that hooks are executed for each child provider, maintaining isolated contexts.
  • Verified before, after, error, and finallyAfter hook stages are called correctly based on evaluation outcomes.
• src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderTest.java
  • Updated shouldHandleDuplicateProviderNames test to assert that MultiProvider correctly deduplicates provider names by appending suffixes (e.g., "provider-1") and that their metadata is correctly stored.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[sonarqubecloud]

Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

[gemini-code-assist]

Code Review

This pull request introduces significant enhancements to the multi-provider functionality, including a new ComparisonStrategy, improved provider name deduplication, event aggregation from child providers, and per-provider hook execution. However, two significant security-related issues were identified: a potential Denial of Service (DoS) vulnerability in the ComparisonStrategy due to inefficient thread pool management, and a violation of the OpenFeature specification regarding hook execution order in the MultiProvider. The hook order issue is particularly concerning as it could lead to security bypasses if hooks are used for authorization or data sanitization. Additionally, a performance consideration in the new ComparisonStrategy and a maintainability point regarding hook execution logic in MultiProvider were noted. These issues should be addressed before merging.

[gemini-code-assist]

The evaluate method creates a new FixedThreadPool for every flag evaluation, which is inefficient and can lead to a Denial of Service (DoS) vulnerability. High traffic could exhaust system resources, causing an OutOfMemoryError and a DoS for the application. It is highly recommended to use a shared, managed ExecutorService instead of creating a new one for every evaluation to prevent performance issues and resource exhaustion.

[gemini-code-assist]

The evaluateWithProviderHooks method violates the OpenFeature specification by executing the before stage of hooks in reverse order, which could lead to security bypasses if hooks are used for authorization or data sanitization. Additionally, this method appears to duplicate hook execution logic, impacting maintainability. The before stage of hooks must be executed in the order they were registered.

Suggested change

	for (int i = hooks.size() - 1; i >= 0; i--) {
	HookExecution<T> execution = hooks.get(i);
	for (HookExecution<T> execution : hooks) {

[gemini-code-assist]

The after stage of hooks must be executed in the reverse order they were registered, according to the OpenFeature specification. The current implementation executes them in forward order.

                for (int i = hooks.size() - 1; i >= 0; i--) {
                    HookExecution<T> execution = hooks.get(i);

[gemini-code-assist]

The error stage of hooks must be executed in the reverse order they were registered, according to the OpenFeature specification. The current implementation executes them in forward order.

                for (int i = hooks.size() - 1; i >= 0; i--) {
                    HookExecution<T> execution = hooks.get(i);

[gemini-code-assist]

The finallyAfter stage of hooks must be executed in the reverse order they were registered, according to the OpenFeature specification. The current implementation executes them in forward order.

            for (int i = hooks.size() - 1; i >= 0; i--) {
                HookExecution<T> execution = hooks.get(i);