Navigation Menu

Skip to content

Commit

Permalink
select: change Symbol handling in script syntax
Browse files Browse the repository at this point in the history
This is a backward incompatible change. But the use case affected by
this change will make Ruby scripts difficult to maintain. I hope that
existing users didn't match the use case. The use case is using Symbol
as string value in script syntax like the following:

    filter("title", :value) # => --filter "title == \"value\""

Before this change, all Symbol are treated as string value in script
syntax. We can't specify column (object in general) in script syntax
with the specification.

With this change, Symbol is treated as identifier. It means that Symbol
is treated as column (object in general) in script syntax like the
following:

    filter("title", :normalized_title) # => --filter "title == normalized_title"

For backward compatibility, invalid identifier in script syntax is
fallbacked to string value in script syntax like the following:

    filter("title", :"Hello World") # => --filter "title == \"Hello World\""

I think that an exception should be raised for the case but I choose the
above specification...
  • Loading branch information
kou committed Apr 27, 2017
1 parent ab55abb commit 8ff9649
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 13 deletions.
13 changes: 12 additions & 1 deletion lib/groonga/client/request/select.rb
Expand Up @@ -433,7 +433,11 @@ def escape_script_syntax_value(value)
when String
ScriptSyntax.format_string(value)
when Symbol
ScriptSyntax.format_string(value.to_s)
if valid_script_syntax_identifier?(value)
value.to_s
else
ScriptSyntax.format_string(value.to_s)
end
when ::Array
escaped_value = "["
value.each_with_index do |element, i|
Expand All @@ -456,6 +460,13 @@ def escape_script_syntax_value(value)
value
end
end

identifier_part = "[a-zA-Z_][a-zA-Z0-9_]*"
VALID_SCRIPT_SYNTAX_IDENTIFIER_PATTERN =
/\A#{identifier_part}(?:\.#{identifier_part})*\z/
def valid_script_syntax_identifier?(value)
VALID_SCRIPT_SYNTAX_IDENTIFIER_PATTERN === value.to_s
end
end

# @private
Expand Down
19 changes: 14 additions & 5 deletions test/request/select/test-filter-equal-parameter.rb
Expand Up @@ -51,11 +51,20 @@ def test_string
to_parameters("title", "[\"He\\ llo\"]"))
end

def test_symbol
assert_equal({
:filter => "title == \"Hello\"",
},
to_parameters("title", :Hello))
sub_test_case("Symbol") do
def test_id
assert_equal({
:filter => "title == normalized_title",
},
to_parameters("title", :normalized_title))
end

def test_not_id
assert_equal({
:filter => "title == \"Hello World\"",
},
to_parameters("title", :"Hello World"))
end
end

def test_number
Expand Down
23 changes: 17 additions & 6 deletions test/request/select/test-filter-expression-parameter.rb
Expand Up @@ -55,12 +55,23 @@ def test_string
:value => "[\"He\\ llo\"]"))
end

def test_symbol
assert_equal({
:filter => "title == \"Hello\"",
},
to_parameters("title == %{value}",
:value => :Hello))
sub_test_case("Symbol") do
def test_valid_id
assert_equal({
:filter => "title == \"Hello\"",
},
to_parameters("%{column} == %{value}",
:column => :title,
:value => "Hello"))
end

def test_invalid_id
assert_equal({
:filter => "title == \"Hello World\"",
},
to_parameters("title == %{value}",
:value => :"Hello World"))
end
end

def test_number
Expand Down
4 changes: 3 additions & 1 deletion test/request/test-select.rb
Expand Up @@ -69,7 +69,9 @@ def filter(*args)
:table => "posts",
:filter => "title == \"Hello\"",
},
filter("title == %{title}", :title => :Hello))
filter("%{column} == %{value}",
:column => :title,
:value => "Hello"))
end

test("Array") do
Expand Down

0 comments on commit 8ff9649

Please sign in to comment.