[WIP] Add choice_rule payload validation#187
Conversation
|
|
||
| def initialize(*) | ||
| super | ||
| @compare_key = payload.keys.detect { |key| key.match?(/^(#{COMPARE_KEYS.join("|")})/) } |
There was a problem hiding this comment.
TODO I don't like the regular expression matcher here because it actually matches on typos like "StringEqualz" (this was my first spec). This is fine when we have the big case compare_key in true? since the else raises an exception but I want it to be stricter in the payload initialize validation.
agrare
force-pushed
the
choice_rule_payload_validation
branch
from
0b56c03 to
88457eb
Compare
agrare
force-pushed
the
choice_rule_payload_validation
branch
2 times, most recently
from
a8e11cd to
da2c0c7
Compare
agrare
force-pushed
the
choice_rule_payload_validation
branch
from
da2c0c7 to
fef939e
Compare
|
Checked commit agrare@fef939e with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint lib/floe/workflow/choice_rule/data.rb
|
kbrock
left a comment
kbrock
left a comment
There was a problem hiding this comment.
I like the attr_reader approach. very nice
| it "raises an exception" do | ||
| expect { subject }.to raise_exception(Floe::InvalidWorkflowError, "Data-test Expression Choice Rule must have a compare key") | ||
| end | ||
| end |
There was a problem hiding this comment.
context "with an invalid compare key" do
let(:payload) { {"Variable" => "$.foo", "InvalidCompare" => "$.bar", "Next" => "FirstMatchState"} }
let(:input) { {"foo" => 0, "bar" => 1} }
it "fails" do
expect { subject }.to raise_exception(Floe::InvalidWorkflowError)
end
endThere was a problem hiding this comment.
👍 yeah I added two more tests one with a typo'd key which would have succeeded on the prior regex approach, and one testing if you have multiple compare keys
| COMPARE_KEYS = ( | ||
| %w[IsNull IsPresent IsNumeric IsString IsBoolean IsTimestamp StringMatches] + | ||
| %w[String Numeric Boolean Timestamp].flat_map { |k| ["#{k}Equals", "#{k}EqualsPath"] } + | ||
| %w[String Numeric Timestamp].flat_map { |k| %w[LessThan GreaterThan LessThanEquals GreaterThanEquals].flat_map { |x| ["#{k}#{x}", "#{k}#{x}Path"] } } | ||
| ).freeze |
There was a problem hiding this comment.
would have preferred using a hash mapping the string to the operation.
Then we could drop the switch
But this is good for now
There was a problem hiding this comment.
That's a good idea let me give that a shot
|
This pull request is not mergeable. Please rebase and repush. |
|
This pull request has been automatically marked as stale because it has not been updated for at least 3 months. If these changes are still valid, please remove the |
2 similar comments
|
This pull request has been automatically marked as stale because it has not been updated for at least 3 months. If these changes are still valid, please remove the |
|
This pull request has been automatically marked as stale because it has not been updated for at least 3 months. If these changes are still valid, please remove the |
3 participants
No description provided.