[FSSDK-11459] Ruby - Add SDK Multi-Region Support for Data Hosting #365
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds multi-region support for data hosting by introducing a region attribute across the SDK, selecting region-specific event endpoints, and updating tests accordingly.
- Introduce
regioninDatafileProjectConfigwith default'US' - Update
EventBuilderandEventFactoryto choose endpoints from a frozenENDPOINTSmap keyed by region - Add or update test cases to include
regionin parameters and expect region-specific URLs
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/spec_params.rb | Add default 'region' key to valid config body |
| spec/project_spec.rb | Include region: 'US' in expected event parameters |
| spec/optimizely_user_context_spec.rb | Add region to expected impression payload |
| spec/event_builder_spec.rb | Introduce @expected_endpoints hash and US region |
| spec/event/user_event_factory_spec.rb | Assert event_context[:region] equals config region |
| spec/event/forwarding_event_processor_spec.rb | Include region: 'US' in forwarding payload |
| spec/event/event_factory_spec.rb | Introduce @expected_endpoints and US region |
| spec/event/event_entities_spec.rb | Include region: 'US' in entity JSON |
| spec/config/datafile_project_config_spec.rb | Add tests for default and specified region |
| lib/optimizely/project_config.rb | Stubbed def region added |
| lib/optimizely/event_builder.rb | Add region param to common params and endpoints |
| lib/optimizely/event/user_event_factory.rb | Pass region into EventContext |
| lib/optimizely/event/event_factory.rb | Add region-based endpoints in log event creation |
| lib/optimizely/event/entity/event_context.rb | Store and serialize region field |
| lib/optimizely/event/entity/event_batch.rb | Track and default region in batch builder |
| lib/optimizely/config/datafile_project_config.rb | Parse and default region on initialization |
Comments suppressed due to low confidence (3)
spec/config/datafile_project_config_spec.rb:771
- [nitpick] The test uses both
config_body_JSONandconfig_body_jsonfor JSON payloads. Consider unifying the variable naming style for consistency.
config_body_json = JSON.dump(config_body_eu)
spec/event_builder_spec.rb:42
- You’ve defined EU endpoints in the test fixture but only assert US behavior. Add test cases to cover the EU region selection path.
@expected_endpoints = {
spec/event/event_factory_spec.rb:37
- The fixture includes EU endpoints, but there’s no assertion for EU region. Add tests to verify that
EventFactorypicks the EU URL whenregion: 'EU'.
@expected_endpoints = {
| @@ -62,6 +62,8 @@ def host_for_odp; end | |||
|
|
|||
| def all_segments; end | |||
|
|
|||
| def region; end | |||
There was a problem hiding this comment.
The region method is currently a stub and returns nothing. It should return the instance variable, e.g., def region; @region; end.
Copilot uses AI. Check for mistakes.
lib/optimizely/event_builder.rb
Outdated
| event_params = get_common_params(project_config, user_id, attributes) | ||
| impression_params = get_impression_params(project_config, experiment, variation_id) | ||
| event_params[:visitors][0][:snapshots].push(impression_params) | ||
|
|
||
| Event.new(:post, ENDPOINT, event_params, POST_HEADERS) | ||
| endpoint = ENDPOINTS[region.to_sym] |
There was a problem hiding this comment.
Converting region directly to a symbol may fail if the string is lowercase. Normalize the key, e.g., region.to_s.upcase.to_sym to ensure a match in ENDPOINTS.
| endpoint = ENDPOINTS[region.to_sym] | |
| endpoint = ENDPOINTS[region.to_s.upcase.to_sym] |
Copilot uses AI. Check for mistakes.
|
|
||
| builder.with_visitors(visitors) | ||
| event_batch = builder.build | ||
| Event.new(:post, ENDPOINT, event_batch.as_json, POST_HEADERS) | ||
|
|
||
| endpoint = ENDPOINTS[user_context[:region].to_sym] || ENDPOINTS[:US] |
There was a problem hiding this comment.
Symbolizing user_context[:region] as-is can miss the intended key if the casing differs. Consider normalizing with user_context[:region].to_s.upcase.to_sym.
| endpoint = ENDPOINTS[user_context[:region].to_sym] || ENDPOINTS[:US] | |
| endpoint = ENDPOINTS[user_context[:region].to_s.upcase.to_sym] || ENDPOINTS[:US] |
Copilot uses AI. Check for mistakes.
Summary
Test plan
Issues