Skip to content

Commit

Permalink
Merge pull request #65 from jdelStrother/sqlite-tablename-fix
Browse files Browse the repository at this point in the history
Fix update statements on sqlite, and quote all table/column names
  • Loading branch information
pboling committed Apr 29, 2017
2 parents c433855 + c92add7 commit 9812c07
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 63 deletions.
24 changes: 19 additions & 5 deletions lib/flag_shih_tzu.rb
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ def chained_flags_with(column = DEFAULT_COLUMN_NAME, *args)
end

def chained_flags_condition(colmn = DEFAULT_COLUMN_NAME, *args)
%[(#{table_name}.#{colmn} in (#{chained_flags_values(colmn, *args).join(",")}))]
%[(#{flag_full_column_name(table_name, colmn)} in (#{chained_flags_values(colmn, *args).join(",")}))]
end

def flag_keys(colmn = DEFAULT_COLUMN_NAME)
Expand All @@ -287,6 +287,19 @@ def flag_keys(colmn = DEFAULT_COLUMN_NAME)

private

def flag_full_column_name(table, column)
"#{connection.quote_table_name(table)}.#{connection.quote_column_name(column)}"
end

def flag_full_column_name_for_assignment(table, column)
if (ActiveRecord::VERSION::MAJOR <= 3)
# If you're trying to do multi-table updates with Rails < 4, sorry - you're out of luck.
connection.quote_column_name(column)
else
connection.quote_table_name_for_assignment(table, column)
end
end

def flag_value_range_for_column(colmn)
max = flag_mapping[colmn].values.max
Range.new(0, (2 * max) - 1)
Expand Down Expand Up @@ -378,13 +391,13 @@ def sql_condition_for_flag(flag, colmn, enabled = true, custom_table_name = tabl
if flag_options[colmn][:flag_query_mode] == :bit_operator
# use & bit operator directly in the SQL query.
# This has the drawback of not using an index on the flags colum.
%[(#{custom_table_name}.#{colmn} & #{flag_mapping[colmn][flag]} = #{enabled ? flag_mapping[colmn][flag] : 0})]
%[(#{flag_full_column_name(custom_table_name, colmn)} & #{flag_mapping[colmn][flag]} = #{enabled ? flag_mapping[colmn][flag] : 0})]
elsif flag_options[colmn][:flag_query_mode] == :in_list
# use IN() operator in the SQL query.
# This has the drawback of becoming a big query
# when you have lots of flags.
neg = enabled ? "" : "not "
%[(#{custom_table_name}.#{colmn} #{neg}in (#{sql_in_for_flag(flag, colmn).join(",")}))]
%[(#{flag_full_column_name(custom_table_name, colmn)} #{neg}in (#{sql_in_for_flag(flag, colmn).join(",")}))]
else
raise NoSuchFlagQueryModeException
end
Expand All @@ -398,8 +411,9 @@ def sql_in_for_flag(flag, colmn)

def sql_set_for_flag(flag, colmn, enabled = true, custom_table_name = table_name)
check_flag(flag, colmn)
full_name = "#{custom_table_name}.#{colmn}"
"#{full_name} = #{full_name} #{enabled ? "| " : "& ~" }#{flag_mapping[colmn][flag]}"
lhs_name = flag_full_column_name_for_assignment(custom_table_name, colmn)
rhs_name = flag_full_column_name(custom_table_name, colmn)
"#{lhs_name} = #{rhs_name} #{enabled ? "| " : "& ~" }#{flag_mapping[colmn][flag]}"
end

def valid_flag_key?(flag_key)
Expand Down
116 changes: 58 additions & 58 deletions test/flag_shih_tzu_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -269,71 +269,71 @@ class SpaceshipWithInvalidFlagName < ActiveRecord::Base
end

def test_should_define_a_sql_condition_method_for_flag_enabled
assert_equal "(spaceships.flags in (1,3,5,7))",
assert_equal '("spaceships"."flags" in (1,3,5,7))',
Spaceship.warpdrive_condition
assert_equal "(spaceships.flags in (2,3,6,7))",
assert_equal '("spaceships"."flags" in (2,3,6,7))',
Spaceship.shields_condition
assert_equal "(spaceships.flags in (4,5,6,7))",
assert_equal '("spaceships"."flags" in (4,5,6,7))',
Spaceship.electrolytes_condition
end

def test_should_define_a_sql_condition_method_for_flag_enabled_with_missing_flags
assert_equal "(spaceships.flags in (1,3,5,7))",
assert_equal '("spaceships"."flags" in (1,3,5,7))',
SpaceshipWithMissingFlags.warpdrive_condition
assert_equal "(spaceships.flags in (4,5,6,7))",
assert_equal '("spaceships"."flags" in (4,5,6,7))',
SpaceshipWithMissingFlags.electrolytes_condition
end

def test_should_accept_a_table_alias_option_for_sql_condition_method
assert_equal "(old_spaceships.flags in (1,3,5,7))",
assert_equal '("old_spaceships"."flags" in (1,3,5,7))',
Spaceship.warpdrive_condition(table_alias: "old_spaceships")
end

def test_should_define_a_sql_condition_method_for_flag_enabled_with_2_colmns
assert_equal "(spaceships_with_2_custom_flags_column.bits in (1,3))",
assert_equal '("spaceships_with_2_custom_flags_column"."bits" in (1,3))',
SpaceshipWith2CustomFlagsColumn.warpdrive_condition
assert_equal "(spaceships_with_2_custom_flags_column.bits in (2,3))",
assert_equal '("spaceships_with_2_custom_flags_column"."bits" in (2,3))',
SpaceshipWith2CustomFlagsColumn.hyperspace_condition
assert_equal "(spaceships_with_2_custom_flags_column.commanders in (1,3))",
assert_equal '("spaceships_with_2_custom_flags_column"."commanders" in (1,3))',
SpaceshipWith2CustomFlagsColumn.jeanlucpicard_condition
assert_equal "(spaceships_with_2_custom_flags_column.commanders in (2,3))",
assert_equal '("spaceships_with_2_custom_flags_column"."commanders" in (2,3))',
SpaceshipWith2CustomFlagsColumn.dajanatroj_condition
end

def test_should_define_a_sql_condition_method_for_flag_not_enabled
assert_equal "(spaceships.flags not in (1,3,5,7))",
assert_equal '("spaceships"."flags" not in (1,3,5,7))',
Spaceship.not_warpdrive_condition
assert_equal "(spaceships.flags not in (2,3,6,7))",
assert_equal '("spaceships"."flags" not in (2,3,6,7))',
Spaceship.not_shields_condition
assert_equal "(spaceships.flags not in (4,5,6,7))",
assert_equal '("spaceships"."flags" not in (4,5,6,7))',
Spaceship.not_electrolytes_condition
end

def test_should_define_a_sql_condition_method_for_flag_not_enabled_with_missing_flags
assert_equal "(spaceships.flags not in (1,3,5,7))",
assert_equal '("spaceships"."flags" not in (1,3,5,7))',
SpaceshipWithMissingFlags.not_warpdrive_condition
assert_equal "(spaceships.flags not in (4,5,6,7))",
assert_equal '("spaceships"."flags" not in (4,5,6,7))',
SpaceshipWithMissingFlags.not_electrolytes_condition
end

def test_sql_condition_for_flag_with_custom_table_name_and_default_query_mode
assert_equal "(custom_spaceships.flags in (1,3,5,7))",
assert_equal '("custom_spaceships"."flags" in (1,3,5,7))',
Spaceship.send(:sql_condition_for_flag,
:warpdrive,
"flags",
true,
"custom_spaceships")
end
def test_sql_condition_for_flag_with_in_list_query_mode
assert_equal "(spaceships.flags in (1,3))",
assert_equal '("spaceships"."flags" in (1,3))',
SpaceshipWithInListQueryMode.send(:sql_condition_for_flag,
:warpdrive,
"flags",
true,
"spaceships")
end
def test_sql_condition_for_flag_with_bit_operator_query_mode
assert_equal "(spaceships.flags & 1 = 1)",
assert_equal '("spaceships"."flags" & 1 = 1)',
SpaceshipWithBitOperatorQueryMode.send(:sql_condition_for_flag,
:warpdrive,
"flags",
Expand All @@ -345,50 +345,50 @@ def test_sql_in_for_flag
Spaceship.send(:sql_in_for_flag, :warpdrive, "flags")
end
def test_sql_set_for_flag
assert_equal "spaceships.flags = spaceships.flags | 1",
assert_equal '"flags" = "spaceships"."flags" | 1',
Spaceship.send(:sql_set_for_flag, :warpdrive, "flags")
end

def test_should_define_a_sql_condition_method_for_flag_enabled_with_2_colmns_not_enabled
assert_equal "(spaceships_with_2_custom_flags_column.bits not in (1,3))",
assert_equal '("spaceships_with_2_custom_flags_column"."bits" not in (1,3))',
SpaceshipWith2CustomFlagsColumn.not_warpdrive_condition
assert_equal "(spaceships_with_2_custom_flags_column.bits not in (2,3))",
assert_equal '("spaceships_with_2_custom_flags_column"."bits" not in (2,3))',
SpaceshipWith2CustomFlagsColumn.not_hyperspace_condition
assert_equal "(spaceships_with_2_custom_flags_column.commanders not in (1,3))",
assert_equal '("spaceships_with_2_custom_flags_column"."commanders" not in (1,3))',
SpaceshipWith2CustomFlagsColumn.not_jeanlucpicard_condition
assert_equal "(spaceships_with_2_custom_flags_column.commanders not in (2,3))",
assert_equal '("spaceships_with_2_custom_flags_column"."commanders" not in (2,3))',
SpaceshipWith2CustomFlagsColumn.not_dajanatroj_condition
end

def test_should_define_a_sql_condition_method_for_flag_enabled_using_bit_operators
assert_equal "(spaceships.flags & 1 = 1)",
assert_equal '("spaceships"."flags" & 1 = 1)',
SpaceshipWithBitOperatorQueryMode.warpdrive_condition
assert_equal "(spaceships.flags & 2 = 2)",
assert_equal '("spaceships"."flags" & 2 = 2)',
SpaceshipWithBitOperatorQueryMode.shields_condition
end

def test_should_define_a_sql_condition_method_for_flag_not_enabled_using_bit_operators
assert_equal "(spaceships.flags & 1 = 0)",
assert_equal '("spaceships"."flags" & 1 = 0)',
SpaceshipWithBitOperatorQueryMode.not_warpdrive_condition
assert_equal "(spaceships.flags & 2 = 0)",
assert_equal '("spaceships"."flags" & 2 = 0)',
SpaceshipWithBitOperatorQueryMode.not_shields_condition
end

def test_should_define_a_named_scope_for_flag_enabled
assert_where_value "(spaceships.flags in (1,3,5,7))",
assert_where_value '("spaceships"."flags" in (1,3,5,7))',
Spaceship.warpdrive
assert_where_value "(spaceships.flags in (2,3,6,7))",
assert_where_value '("spaceships"."flags" in (2,3,6,7))',
Spaceship.shields
assert_where_value "(spaceships.flags in (4,5,6,7))",
assert_where_value '("spaceships"."flags" in (4,5,6,7))',
Spaceship.electrolytes
end

def test_should_define_a_named_scope_for_flag_not_enabled
assert_where_value "(spaceships.flags not in (1,3,5,7))",
assert_where_value '("spaceships"."flags" not in (1,3,5,7))',
Spaceship.not_warpdrive
assert_where_value "(spaceships.flags not in (2,3,6,7))",
assert_where_value '("spaceships"."flags" not in (2,3,6,7))',
Spaceship.not_shields
assert_where_value "(spaceships.flags not in (4,5,6,7))",
assert_where_value '("spaceships"."flags" not in (4,5,6,7))',
Spaceship.not_electrolytes
end

Expand All @@ -398,38 +398,38 @@ def test_should_define_a_dynamic_column_value_helpers_for_flags
end

def test_should_define_a_named_scope_for_flag_enabled_with_2_columns
assert_where_value "(spaceships_with_2_custom_flags_column.bits in (1,3))",
assert_where_value '("spaceships_with_2_custom_flags_column"."bits" in (1,3))',
SpaceshipWith2CustomFlagsColumn.warpdrive
assert_where_value "(spaceships_with_2_custom_flags_column.bits in (2,3))",
assert_where_value '("spaceships_with_2_custom_flags_column"."bits" in (2,3))',
SpaceshipWith2CustomFlagsColumn.hyperspace
assert_where_value "(spaceships_with_2_custom_flags_column.commanders in (1,3))",
assert_where_value '("spaceships_with_2_custom_flags_column"."commanders" in (1,3))',
SpaceshipWith2CustomFlagsColumn.jeanlucpicard
assert_where_value "(spaceships_with_2_custom_flags_column.commanders in (2,3))",
assert_where_value '("spaceships_with_2_custom_flags_column"."commanders" in (2,3))',
SpaceshipWith2CustomFlagsColumn.dajanatroj
end

def test_should_define_a_named_scope_for_flag_not_enabled_with_2_columns
assert_where_value "(spaceships_with_2_custom_flags_column.bits not in (1,3))",
assert_where_value '("spaceships_with_2_custom_flags_column"."bits" not in (1,3))',
SpaceshipWith2CustomFlagsColumn.not_warpdrive
assert_where_value "(spaceships_with_2_custom_flags_column.bits not in (2,3))",
assert_where_value '("spaceships_with_2_custom_flags_column"."bits" not in (2,3))',
SpaceshipWith2CustomFlagsColumn.not_hyperspace
assert_where_value "(spaceships_with_2_custom_flags_column.commanders not in (1,3))",
assert_where_value '("spaceships_with_2_custom_flags_column"."commanders" not in (1,3))',
SpaceshipWith2CustomFlagsColumn.not_jeanlucpicard
assert_where_value "(spaceships_with_2_custom_flags_column.commanders not in (2,3))",
assert_where_value '("spaceships_with_2_custom_flags_column"."commanders" not in (2,3))',
SpaceshipWith2CustomFlagsColumn.not_dajanatroj
end

def test_should_define_a_named_scope_for_flag_enabled_using_bit_operators
assert_where_value "(spaceships.flags & 1 = 1)",
assert_where_value '("spaceships"."flags" & 1 = 1)',
SpaceshipWithBitOperatorQueryMode.warpdrive
assert_where_value "(spaceships.flags & 2 = 2)",
assert_where_value '("spaceships"."flags" & 2 = 2)',
SpaceshipWithBitOperatorQueryMode.shields
end

def test_should_define_a_named_scope_for_flag_not_enabled_using_bit_operators
assert_where_value "(spaceships.flags & 1 = 0)",
assert_where_value '("spaceships"."flags" & 1 = 0)',
SpaceshipWithBitOperatorQueryMode.not_warpdrive
assert_where_value "(spaceships.flags & 2 = 0)",
assert_where_value '("spaceships"."flags" & 2 = 0)',
SpaceshipWithBitOperatorQueryMode.not_shields
end

Expand Down Expand Up @@ -511,16 +511,16 @@ def test_should_return_the_correct_number_of_items_from_a_named_scope
end

def test_should_return_the_correct_condition_with_chained_flags
assert_equal "(spaceships.flags in (3,7))",
assert_equal '("spaceships"."flags" in (3,7))',
Spaceship.chained_flags_condition("flags",
:warpdrive,
:shields)
assert_equal "(spaceships.flags in (7))",
assert_equal '("spaceships"."flags" in (7))',
Spaceship.chained_flags_condition("flags",
:warpdrive,
:shields,
:electrolytes)
assert_equal "(spaceships.flags in (2,6))",
assert_equal '("spaceships"."flags" in (2,6))',
Spaceship.chained_flags_condition("flags",
:not_warpdrive,
:shields)
Expand Down Expand Up @@ -577,21 +577,21 @@ def test_should_work_with_a_custom_flags_column
spaceship.save!
spaceship.reload
assert_equal 3, spaceship.flags("bits")
assert_equal "(spaceships_with_custom_flags_column.bits in (1,3))",
assert_equal '("spaceships_with_custom_flags_column"."bits" in (1,3))',
SpaceshipWithCustomFlagsColumn.warpdrive_condition
assert_equal "(spaceships_with_custom_flags_column.bits not in (1,3))",
assert_equal '("spaceships_with_custom_flags_column"."bits" not in (1,3))',
SpaceshipWithCustomFlagsColumn.not_warpdrive_condition
assert_equal "(spaceships_with_custom_flags_column.bits in (2,3))",
assert_equal '("spaceships_with_custom_flags_column"."bits" in (2,3))',
SpaceshipWithCustomFlagsColumn.hyperspace_condition
assert_equal "(spaceships_with_custom_flags_column.bits not in (2,3))",
assert_equal '("spaceships_with_custom_flags_column"."bits" not in (2,3))',
SpaceshipWithCustomFlagsColumn.not_hyperspace_condition
assert_where_value %[(spaceships_with_custom_flags_column.bits in (1,3))],
assert_where_value '("spaceships_with_custom_flags_column"."bits" in (1,3))',
SpaceshipWithCustomFlagsColumn.warpdrive
assert_where_value %[(spaceships_with_custom_flags_column.bits not in (1,3))],
assert_where_value '("spaceships_with_custom_flags_column"."bits" not in (1,3))',
SpaceshipWithCustomFlagsColumn.not_warpdrive
assert_where_value %[(spaceships_with_custom_flags_column.bits in (2,3))],
assert_where_value '("spaceships_with_custom_flags_column"."bits" in (2,3))',
SpaceshipWithCustomFlagsColumn.hyperspace
assert_where_value %[(spaceships_with_custom_flags_column.bits not in (2,3))],
assert_where_value '("spaceships_with_custom_flags_column"."bits" not in (2,3))',
SpaceshipWithCustomFlagsColumn.not_hyperspace
end

Expand Down Expand Up @@ -1421,9 +1421,9 @@ def test_should_define_bang_methods
end

def test_should_return_a_sql_set_method_for_flag
assert_equal "spaceships.flags = spaceships.flags | 1",
assert_equal '"flags" = "spaceships"."flags" | 1',
Spaceship.send(:sql_set_for_flag, :warpdrive, "flags", true)
assert_equal "spaceships.flags = spaceships.flags & ~1",
assert_equal '"flags" = "spaceships"."flags" & ~1',
Spaceship.send(:sql_set_for_flag, :warpdrive, "flags", false)
end

Expand Down

0 comments on commit 9812c07

Please sign in to comment.