Skip to content

Commit b099c80

Browse files
authored
Inject bind variables in rails 4 with prepared_statements set to false (zooniverse#3139)
* ensure the travis disables prepared statements non local envs disable prepared statements, manual SQL work relies on testing these code paths. * disable prepared_statemetns for travis testing * switch to sanitize the sql string vs use bind params * replace named bind vars use internal Sanitization method replace_named_bind_variables to safely inject the subject_set_id * This is certainly one way to do it. * test rails 5 AR code paths * remove the panoptes rails5? method
1 parent 23565ba commit b099c80

File tree

4 files changed

+50
-14
lines changed

4 files changed

+50
-14
lines changed

app/workers/subject_metadata_worker.rb

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,43 @@ class SubjectMetadataWorker
22
include Sidekiq::Worker
33

44
def perform(subject_set_id)
5-
update_sms_priority_sql = <<-SQL
6-
UPDATE set_member_subjects
7-
SET priority = CAST(subjects.metadata->>'#priority' AS NUMERIC)
8-
FROM subjects
9-
WHERE subjects.id = set_member_subjects.subject_id
10-
AND subjects.metadata ? '#priority'
11-
AND set_member_subjects.subject_set_id = $1
12-
SQL
13-
14-
ActiveRecord::Base.connection.exec_update(
15-
update_sms_priority_sql,
16-
"SQL",
17-
[[nil, subject_set_id]]
18-
)
5+
if ActiveRecord::VERSION::MAJOR == 5
6+
update_sms_priority_sql = <<-SQL
7+
UPDATE set_member_subjects
8+
SET priority = CAST(subjects.metadata->>'#priority' AS NUMERIC)
9+
FROM subjects
10+
WHERE subjects.id = set_member_subjects.subject_id
11+
AND subjects.metadata ? '#priority'
12+
AND set_member_subjects.subject_set_id = $1
13+
SQL
14+
ActiveRecord::Base.connection.exec_update(
15+
update_sms_priority_sql,
16+
"SQL",
17+
[[nil, subject_set_id]]
18+
)
19+
else
20+
update_sms_priority_sql = <<-SQL
21+
UPDATE set_member_subjects
22+
SET priority = CAST(subjects.metadata->>'#priority' AS NUMERIC)
23+
FROM subjects
24+
WHERE subjects.id = set_member_subjects.subject_id
25+
AND subjects.metadata ? '#priority'
26+
AND set_member_subjects.subject_set_id = :subject_set_id
27+
SQL
28+
# handle incorrect bind params for non preparted statements
29+
# in exec_update, fixed in rails 5
30+
# https://github.com/rails/rails/issues/24893
31+
# https://github.com/rails/rails/issues/34183
32+
bound_update_sms_priority_sql = ActiveRecord::Base.send(
33+
:replace_named_bind_variables,
34+
update_sms_priority_sql,
35+
{subject_set_id: subject_set_id}
36+
)
37+
ActiveRecord::Base.connection.exec_update(
38+
bound_update_sms_priority_sql,
39+
"SQL",
40+
[]
41+
)
42+
end
1943
end
2044
end

config/database.yml.hudson

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ default: &default
77
host: pg
88
pool: 5
99
port: 5432
10+
prepared_statements: false
1011

1112
development:
1213
<<: *default

lib/tasks/configure.rake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ test:
3131
adapter: postgresql
3232
database: travis_ci_test
3333
username: postgres
34+
prepared_statements: false
3435
YAML
3536

3637
File.open('config/redis.yml', 'w') { |f| f.write(<<YAML) }

spec/workers/subject_metadata_worker_spec.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,16 @@
1717
end
1818

1919
describe "#perform" do
20+
# TODO: Rails 5 combine the tests to one
21+
# to test behaviour not AR calling interface
22+
it 'calls the correct RAILS 5 AR methods' do
23+
stub_const("ActiveRecord::VERSION::MAJOR", 5)
24+
expect(ActiveRecord::Base.connection)
25+
.to receive(:exec_update)
26+
.with(instance_of(String), "SQL",[[nil, subject_set.id]])
27+
worker.perform(subject_set.id)
28+
end
29+
2030
it 'copies priority from metadata to SMS attribute' do
2131
worker.perform(subject_set.id)
2232
sms_one, sms_two, sms_three = SetMemberSubject.find(

0 commit comments

Comments
 (0)