refactor: odp graphql query (#317)

* make graphql query more robust
* rename odp api managers

Author: andrewleap-optimizely

lib/optimizely/odp/odp_events_api_manager.rb renamed to lib/optimizely/odp/odp_event_api_manager.rb

@@ -19,7 +19,7 @@
 require 'json'
 
 module Optimizely
-  class OdpEventsApiManager
+  class OdpEventApiManager
     # Interface that handles sending ODP events.
 
     def initialize(logger: nil, proxy_config: nil)

lib/optimizely/odp/odp_event_manager.rb

@@ -15,7 +15,7 @@
 #    See the License for the specific language governing permissions and
 #    limitations under the License.
 #
-require_relative 'odp_events_api_manager'
+require_relative 'odp_event_api_manager'
 require_relative '../helpers/constants'
 require_relative 'odp_event'
 
@@ -47,7 +47,7 @@ def initialize(
       # received signal should be sent after adding item to event_queue
       @received = ConditionVariable.new
       @logger = logger
-      @api_manager = api_manager || OdpEventsApiManager.new(logger: @logger, proxy_config: proxy_config)
+      @api_manager = api_manager || OdpEventApiManager.new(logger: @logger, proxy_config: proxy_config)
       @batch_size = Helpers::Constants::ODP_EVENT_MANAGER[:DEFAULT_BATCH_SIZE]
       @flush_interval = Helpers::Constants::ODP_EVENT_MANAGER[:DEFAULT_FLUSH_INTERVAL_SECONDS]
       @flush_deadline = 0

lib/optimizely/odp/odp_segments_api_manager.rb renamed to lib/optimizely/odp/odp_segment_api_manager.rb

@@ -19,7 +19,7 @@
 require 'json'
 
 module Optimizely
-  class OdpSegmentsApiManager
+  class OdpSegmentApiManager
     # Interface that handles fetching audience segments.
 
     def initialize(logger: nil, proxy_config: nil)
@@ -41,8 +41,13 @@ def fetch_segments(api_key, api_host, user_key, user_value, segments_to_check)
       headers = {'Content-Type' => 'application/json', 'x-api-key' => api_key.to_s}
 
       payload = {
-        'query' => %'query {customer(#{user_key}: "#{user_value}")' \
-                   "{audiences(subset:#{segments_to_check || '[]'}) {edges {node {name state}}}}}"
+        query: 'query($userId: String, $audiences: [String]) {' \
+               "customer(#{user_key}: $userId) " \
+               '{audiences(subset: $audiences) {edges {node {name state}}}}}',
+        variables: {
+          userId: user_value.to_s,
+          audiences: segments_to_check || []
+        }
       }.to_json
 
       begin

lib/optimizely/odp/odp_segment_manager.rb

@@ -17,7 +17,7 @@
 #
 
 require 'optimizely/logger'
-require_relative 'odp_segments_api_manager'
+require_relative 'odp_segment_api_manager'
 
 module Optimizely
   class OdpSegmentManager
@@ -28,7 +28,7 @@ class OdpSegmentManager
     def initialize(segments_cache, api_manager = nil, logger = nil, proxy_config = nil)
       @odp_config = nil
       @logger = logger || NoOpLogger.new
-      @api_manager = api_manager || OdpSegmentsApiManager.new(logger: @logger, proxy_config: proxy_config)
+      @api_manager = api_manager || OdpSegmentApiManager.new(logger: @logger, proxy_config: proxy_config)
       @segments_cache = segments_cache
     end
 

spec/odp/odp_events_api_manager_spec.rb renamed to spec/odp/odp_event_api_manager_spec.rb

@@ -16,9 +16,9 @@
 #    limitations under the License.
 #
 require 'spec_helper'
-require 'optimizely/odp/odp_events_api_manager'
+require 'optimizely/odp/odp_event_api_manager'
 
-describe Optimizely::OdpEventsApiManager do
+describe Optimizely::OdpEventApiManager do
   let(:user_key) { 'vuid' }
   let(:user_value) { 'test-user-value' }
   let(:api_key) { 'test-api-key' }
@@ -47,7 +47,7 @@
           body: events.to_json
         ).to_return(status: 200)
 
-      api_manager = Optimizely::OdpEventsApiManager.new
+      api_manager = Optimizely::OdpEventApiManager.new
       expect(spy_logger).not_to receive(:log)
       should_retry = api_manager.send_odp_events(api_key, api_host, events)
 
@@ -56,7 +56,7 @@
 
     it 'should return true on network error' do
       allow(Optimizely::Helpers::HttpUtils).to receive(:make_request).and_raise(SocketError)
-      api_manager = Optimizely::OdpEventsApiManager.new(logger: spy_logger)
+      api_manager = Optimizely::OdpEventApiManager.new(logger: spy_logger)
       expect(spy_logger).to receive(:log).with(Logger::ERROR, 'ODP event send failed (network error).')
 
       should_retry = api_manager.send_odp_events(api_key, api_host, events)
@@ -70,7 +70,7 @@
           body: events.to_json
         ).to_return(status: [400, 'Bad Request'], body: failure_response_data)
 
-      api_manager = Optimizely::OdpEventsApiManager.new(logger: spy_logger)
+      api_manager = Optimizely::OdpEventApiManager.new(logger: spy_logger)
       expect(spy_logger).to receive(:log).with(
         Logger::ERROR, 'ODP event send failed ({"title":"Bad Request","status":400,' \
                        '"timestamp":"2022-07-01T20:44:00.945Z","detail":{"invalids":' \
@@ -88,7 +88,7 @@
           body: events.to_json
         ).to_return(status: [500, 'Internal Server Error'])
 
-      api_manager = Optimizely::OdpEventsApiManager.new(logger: spy_logger)
+      api_manager = Optimizely::OdpEventApiManager.new(logger: spy_logger)
       expect(spy_logger).to receive(:log).with(Logger::ERROR, 'ODP event send failed (500: Internal Server Error).')
 
       should_retry = api_manager.send_odp_events(api_key, api_host, events)

spec/odp/odp_event_manager_spec.rb

@@ -17,7 +17,7 @@
 require 'optimizely/odp/odp_event'
 require 'optimizely/odp/lru_cache'
 require 'optimizely/odp/odp_config'
-require 'optimizely/odp/odp_events_api_manager'
+require 'optimizely/odp/odp_event_api_manager'
 require 'optimizely/logger'
 require 'optimizely/helpers/validator'
 
@@ -98,7 +98,7 @@
     it 'should return OdpEventManager instance' do
       config = Optimizely::OdpConfig.new
 
-      api_manager = Optimizely::OdpEventsApiManager.new
+      api_manager = Optimizely::OdpEventApiManager.new
       event_manager = Optimizely::OdpEventManager.new(api_manager: api_manager, logger: spy_logger)
       event_manager.start!(config)
 
@@ -109,7 +109,7 @@
 
       event_manager = Optimizely::OdpEventManager.new
       expect(event_manager.logger).to be_a Optimizely::NoOpLogger
-      expect(event_manager.api_manager).to be_a Optimizely::OdpEventsApiManager
+      expect(event_manager.api_manager).to be_a Optimizely::OdpEventApiManager
     end
   end
 

spec/odp/odp_manager_spec.rb

@@ -18,7 +18,7 @@
 require 'optimizely/odp/odp_event'
 require 'optimizely/odp/lru_cache'
 require 'optimizely/odp/odp_config'
-require 'optimizely/odp/odp_events_api_manager'
+require 'optimizely/odp/odp_event_api_manager'
 require 'optimizely/logger'
 require 'optimizely/helpers/validator'
 require 'optimizely/helpers/constants'

spec/odp/odp_segments_api_manager_spec.rb renamed to spec/odp/odp_segment_api_manager_spec.rb

@@ -16,16 +16,21 @@
 #    limitations under the License.
 #
 require 'spec_helper'
-require 'optimizely/odp/odp_segments_api_manager'
+require 'optimizely/odp/odp_segment_api_manager'
 
-describe Optimizely::OdpSegmentsApiManager do
+describe Optimizely::OdpSegmentApiManager do
   let(:user_key) { 'vuid' }
   let(:user_value) { 'test-user-value' }
   let(:api_key) { 'test-api-key' }
   let(:api_host) { 'https://test-host' }
   let(:error_handler) { Optimizely::RaiseErrorHandler.new }
   let(:spy_logger) { spy('logger') }
-  let(:api_manager) { Optimizely::OdpSegmentsApiManager.new(logger: spy_logger) }
+  let(:api_manager) { Optimizely::OdpSegmentApiManager.new(logger: spy_logger) }
+  let(:graphql_query) do
+    'query($userId: String, $audiences: [String]) {' \
+    "customer(#{user_key}: $userId) " \
+    '{audiences(subset: $audiences) {edges {node {name state}}}}}'
+  end
   let(:good_response_data) do
     {
       data: {
@@ -221,10 +226,7 @@
       stub_request(:post, "#{api_host}/v3/graphql")
         .with(
           headers: {'content-type': 'application/json', 'x-api-key': api_key},
-          body: {
-            query: %'query {customer(#{user_key}: "#{user_value}")' \
-            '{audiences(subset:["a", "b", "c"]) {edges {node {name state}}}}}'
-          }
+          body: {query: graphql_query, variables: {userId: user_value, audiences: %w[a b c]}}
         )
         .to_return(status: 200, body: good_response_data.to_json)
 
@@ -390,47 +392,27 @@
 
     it 'should create correct subset filter' do
       stub_request(:post, "#{api_host}/v3/graphql")
-        .with(
-          body: {
-            query: %'query {customer(#{user_key}: "#{user_value}")' \
-            '{audiences(subset:[]) {edges {node {name state}}}}}'
-          }
-        )
+        .with(body: {query: graphql_query, variables: {userId: user_value, audiences: []}})
       api_manager.fetch_segments(api_key, api_host, user_key, user_value, nil)
 
       stub_request(:post, "#{api_host}/v3/graphql")
-        .with(
-          body: {
-            query: %'query {customer(#{user_key}: "#{user_value}")' \
-            '{audiences(subset:[]) {edges {node {name state}}}}}'
-          }
-        )
+        .with(body: {query: graphql_query, variables: {userId: user_value, audiences: []}})
       api_manager.fetch_segments(api_key, api_host, user_key, user_value, [])
 
       stub_request(:post, "#{api_host}/v3/graphql")
-        .with(
-          body: {
-            query: %'query {customer(#{user_key}: "#{user_value}")' \
-            '{audiences(subset:["a"]) {edges {node {name state}}}}}'
-          }
-        )
+        .with(body: {query: graphql_query, variables: {userId: user_value, audiences: %w[a]}})
       api_manager.fetch_segments(api_key, api_host, user_key, user_value, %w[a])
 
       stub_request(:post, "#{api_host}/v3/graphql")
-        .with(
-          body: {
-            query: %'query {customer(#{user_key}: "#{user_value}")' \
-            '{audiences(subset:["a", "b", "c"]) {edges {node {name state}}}}}'
-          }
-        )
+        .with(body: {query: graphql_query, variables: {userId: user_value, audiences: %w[a b c]}})
       api_manager.fetch_segments(api_key, api_host, user_key, user_value, %w[a b c])
     end
 
     it 'should pass the proxy config that is passed in' do
       allow(Optimizely::Helpers::HttpUtils).to receive(:make_request).and_raise(SocketError)
       stub_request(:post, "#{api_host}/v3/graphql")
 
-      api_manager = Optimizely::OdpSegmentsApiManager.new(logger: spy_logger, proxy_config: :proxy_config)
+      api_manager = Optimizely::OdpSegmentApiManager.new(logger: spy_logger, proxy_config: :proxy_config)
       api_manager.fetch_segments(api_key, api_host, user_key, user_value, [])
       expect(Optimizely::Helpers::HttpUtils).to have_received(:make_request).with(anything, anything, anything, anything, anything, :proxy_config)
     end

spec/odp/odp_segment_manager_spec.rb

@@ -16,7 +16,7 @@
 require 'optimizely/odp/odp_segment_manager'
 require 'optimizely/odp/lru_cache'
 require 'optimizely/odp/odp_config'
-require 'optimizely/odp/odp_segments_api_manager'
+require 'optimizely/odp/odp_segment_api_manager'
 require 'optimizely/logger'
 
 describe Optimizely::OdpSegmentManager do
@@ -63,7 +63,7 @@
 
   describe '#initialize' do
     it 'should return OdpSegmentManager instance' do
-      api_manager = Optimizely::OdpSegmentsApiManager.new
+      api_manager = Optimizely::OdpSegmentApiManager.new
       segment_manager = Optimizely::OdpSegmentManager.new(segments_cache, api_manager, spy_logger)
 
       expect(segment_manager.segments_cache).to be_a Optimizely::LRUCache
@@ -74,16 +74,18 @@
 
       segment_manager = Optimizely::OdpSegmentManager.new(segments_cache)
       expect(segment_manager.logger).to be_a Optimizely::NoOpLogger
-      expect(segment_manager.api_manager).to be_a Optimizely::OdpSegmentsApiManager
+      expect(segment_manager.api_manager).to be_a Optimizely::OdpSegmentApiManager
     end
   end
 
   describe '#fetch_qualified_segments' do
     it 'should return segments successfully' do
       stub_request(:post, "#{api_host}/v3/graphql")
         .with({headers: {'x-api-key': api_key}, body: {
-                'query' => %'query {customer(#{user_key}: "#{user_value}")' \
-                "{audiences(subset:#{segments_to_check}) {edges {node {name state}}}}}"
+                query: 'query($userId: String, $audiences: [String]) {' \
+                       "customer(#{user_key}: $userId) " \
+                       '{audiences(subset: $audiences) {edges {node {name state}}}}}',
+                variables: {userId: user_value, audiences: %w[a b c]}
               }})
         .to_return(status: 200, body: good_response_data)