Skip to content

Commit 13d4aeb

Browse files
committed
Address code review feedback
1 parent 917ab65 commit 13d4aeb

4 files changed

Lines changed: 28 additions & 34 deletions

File tree

app/models/semantic_query_builder.rb

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
require 'aws-sdk-lambda'
2-
31
class SemanticQueryBuilder
42
def build(params, fulltext: false)
53
query_text = params[:q].to_s.strip
@@ -14,17 +12,10 @@ def build(params, fulltext: false)
1412
private
1513

1614
def invoke_semantic_builder(query_text)
17-
client_options = {
18-
region: ENV.fetch('AWS_REGION', 'us-east-1'),
19-
access_key_id: ENV.fetch('AWS_ACCESS_KEY_ID', nil),
20-
secret_access_key: ENV.fetch('AWS_SECRET_ACCESS_KEY', nil)
21-
}
22-
23-
client = Aws::Lambda::Client.new(client_options)
2415
payload = { query: query_text }
2516

26-
response = client.invoke(
27-
function_name: ENV.fetch('TIMDEX_SEMANTIC_BUILDER_FUNCTION_NAME', nil),
17+
response = Timdex::LambdaClient.invoke(
18+
function_name: ENV.fetch('TIMDEX_SEMANTIC_BUILDER_FUNCTION_NAME'),
2819
invocation_type: 'RequestResponse',
2920
payload: payload.to_json
3021
)

config/initializers/lambda.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
require 'aws-sdk-lambda'
2+
3+
def configure_lambda_client
4+
Aws::Lambda::Client.new(
5+
region: ENV.fetch('AWS_REGION', 'us-east-1'),
6+
access_key_id: ENV.fetch('AWS_ACCESS_KEY_ID'),
7+
secret_access_key: ENV.fetch('AWS_SECRET_ACCESS_KEY')
8+
)
9+
end
10+
11+
Timdex::LambdaClient = configure_lambda_client

test/models/opensearch_test.rb

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -115,22 +115,12 @@ class OpensearchTest < ActiveSupport::TestCase
115115
os.instance_variable_set(:@fulltext, false)
116116
os.instance_variable_set(:@query_mode, 'keyword')
117117

118+
expected_query = { bool: { must: [{ match: { text: { query: 'test' } } }] } }
119+
mock_builder = mock
120+
mock_builder.stubs(:build).returns(expected_query)
121+
LexicalQueryBuilder.expects(:new).once.returns(mock_builder)
118122
result = os.query
119123

120-
# Verify lexical builder returns a bool query
121-
assert result.is_a?(Hash)
122-
assert_includes(result.keys, :bool)
123-
end
124-
125-
test 'uses LexicalQueryBuilder by default when no queryMode given' do
126-
os = Opensearch.new
127-
os.instance_variable_set(:@params, { q: 'test' })
128-
os.instance_variable_set(:@fulltext, false)
129-
os.instance_variable_set(:@query_mode, nil)
130-
131-
result = os.query
132-
133-
# Verify lexical builder returns a bool query
134124
assert result.is_a?(Hash)
135125
assert_includes(result.keys, :bool)
136126
end
@@ -141,10 +131,10 @@ class OpensearchTest < ActiveSupport::TestCase
141131
os.instance_variable_set(:@fulltext, false)
142132
os.instance_variable_set(:@query_mode, 'semantic')
143133

144-
# Mock SemanticQueryBuilder to avoid actual Lambda call
145134
mock_response = { 'bool' => { 'should' => [{ 'rank_feature' => { 'field' => 'test', 'boost' => 1.0 } }] } }
146-
147-
SemanticQueryBuilder.any_instance.stubs(:build).returns(mock_response)
135+
mock_builder = mock
136+
mock_builder.stubs(:build).returns(mock_response)
137+
SemanticQueryBuilder.expects(:new).once.returns(mock_builder)
148138

149139
result = os.query
150140
assert_equal(mock_response, result)

test/models/semantic_query_builder_test.rb

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ class SemanticQueryBuilderTest < ActiveSupport::TestCase
55
@builder = SemanticQueryBuilder.new
66
end
77

8-
def mock_lambda_invoke(response_data)
9-
# Use StringIO to simulate real AWS SDK behavior (payload is IO-like, not a plain string)
8+
# Sets up a mock Lambda response for testing semantic query builder.
9+
# Mocks Aws::Lambda::Client#invoke to return the provided response_data, simulating AWS SDK
10+
# payload (StringIO).
11+
def setup_mock_lambda(response_data)
1012
mock_response = Struct.new(:payload).new(StringIO.new(response_data.to_json))
1113
Aws::Lambda::Client.any_instance.expects(:invoke).returns(mock_response)
1214
end
@@ -34,7 +36,7 @@ def mock_lambda_invoke(response_data)
3436

3537
test 'builds semantic query from lambda response' do
3638
query_text = 'hello world'
37-
expected_response = {
39+
mock_response = {
3840
'query' => {
3941
'bool' => {
4042
'should' => [
@@ -45,7 +47,7 @@ def mock_lambda_invoke(response_data)
4547
}
4648
}
4749

48-
mock_lambda_invoke(expected_response)
50+
setup_mock_lambda(mock_response)
4951

5052
params = { q: query_text }
5153
result = @builder.build(params)
@@ -77,7 +79,7 @@ def mock_lambda_invoke(response_data)
7779
query_text = 'no query key'
7880
invalid_response = { 'result' => {} }
7981

80-
mock_lambda_invoke(invalid_response)
82+
setup_mock_lambda(invalid_response)
8183

8284
params = { q: query_text }
8385

@@ -90,7 +92,7 @@ def mock_lambda_invoke(response_data)
9092
query_text = 'invalid query type'
9193
invalid_response = { 'query' => 'not a hash' }
9294

93-
mock_lambda_invoke(invalid_response)
95+
setup_mock_lambda(invalid_response)
9496

9597
params = { q: query_text }
9698

0 commit comments

Comments
 (0)