-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Add Intrinsic Functions #84
Closed
Closed
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
# frozen_string_literal: true | ||
|
||
module Floe | ||
class Workflow | ||
class IntrinsicFunction | ||
INTRINSIC_FUNCTION_REGEX = /^(?<module>\w+)\.(?<function>\w+)\((?<args>.*)\)$/.freeze | ||
|
||
class << self | ||
def new(*args) | ||
if self == Floe::Workflow::IntrinsicFunction | ||
detect_class(*args).new(*args) | ||
else | ||
super | ||
end | ||
end | ||
|
||
private def detect_class(payload) | ||
function_module_name, function_name = | ||
payload.match(INTRINSIC_FUNCTION_REGEX) | ||
.named_captures | ||
.values_at("module", "function") | ||
|
||
begin | ||
function_module = Floe::Workflow::IntrinsicFunctions.const_get(function_module_name) | ||
function_module.const_get(function_name) | ||
rescue NameError | ||
raise NotImplementedError, "#{function_module_name}.#{function_name} is not implemented" | ||
end | ||
end | ||
|
||
def value(payload, context, input = {}) | ||
new(payload).value(context, input) | ||
end | ||
end | ||
|
||
attr_reader :args | ||
|
||
def initialize(payload) | ||
args = payload.match(INTRINSIC_FUNCTION_REGEX).named_captures["args"] | ||
@args = args.split(", ").map do |arg| | ||
if arg.start_with?("$.") | ||
Path.new(arg) | ||
elsif arg.match?(INTRINSIC_FUNCTION_REGEX) | ||
Floe::Workflow::IntrinsicFunction.new(arg) | ||
else | ||
arg | ||
end | ||
end | ||
end | ||
|
||
def value(_context, _inputs) | ||
raise NotImplementedError, "must be implemented in a subclass" | ||
end | ||
end | ||
end | ||
end |
18 changes: 18 additions & 0 deletions
18
lib/floe/workflow/intrinsic_functions/states/string_to_json.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
# frozen_string_literal: true | ||
|
||
module Floe | ||
class Workflow | ||
module IntrinsicFunctions | ||
module States | ||
class StringToJson < Floe::Workflow::IntrinsicFunction | ||
def value(context, inputs) | ||
arg = args.first | ||
arg = arg.value(context, inputs) if arg.respond_to?(:value) | ||
|
||
JSON.parse(arg) | ||
end | ||
end | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
RSpec.describe Floe::Workflow::IntrinsicFunction do | ||
describe "#initialize" do | ||
let(:payload) { "States.StringToJson()" } | ||
|
||
it "returns an instance of the intrinsic function" do | ||
expect(described_class.new(payload)).to be_kind_of(Floe::Workflow::IntrinsicFunctions::States::StringToJson) | ||
end | ||
|
||
context "with an invalid intrinsic function" do | ||
let(:payload) { "States.MyFirstFunction()" } | ||
|
||
it "raises an exception for an invalid intrinsic function" do | ||
expect { described_class.new(payload) }.to raise_error(NotImplementedError) | ||
end | ||
end | ||
|
||
context "with no arguments" do | ||
it "parses args as empty" do | ||
function = described_class.new(payload) | ||
expect(function.args).to be_empty | ||
end | ||
end | ||
|
||
context "with a string arguments" do | ||
let(:payload) { "States.StringToJson(foobar)" } | ||
|
||
it "parses the arguments" do | ||
function = described_class.new(payload) | ||
expect(function.args).to eq(["foobar"]) | ||
end | ||
end | ||
|
||
context "with a Path" do | ||
let(:payload) { "States.StringToJson($.someString)" } | ||
|
||
it "parses the arguments" do | ||
function = described_class.new(payload) | ||
expect(function.args.first).to be_kind_of(Floe::Workflow::Path) | ||
end | ||
end | ||
|
||
context "with an IntrinsicFunction" do | ||
let(:payload) { "States.StringToJson(States.StringToJson($.someString))" } | ||
|
||
it "parses the arguments" do | ||
function = described_class.new(payload) | ||
expect(function.args.first).to be_kind_of(Floe::Workflow::IntrinsicFunction) | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have one concern here. I was looking through the list of Intrinsic Functions and there is one called
States.Array
. This would imply we have aFloe::Workflow::IntrinsicFunctions::States::Array
class, and thatArray
shadowing kind of scares me (regardless of const_get or literal). I'm wondering if instead of a full namespacing, we instead just smash the module/function together so you'd get something more like:Floe::Workflow::IntrinsicFunctions::StatesArray
. Similarly, there is aStates::Hash
class. Thoughts?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm we could have
States
be a class with methodsdef StringToJson
anddef Array
rather than having classes for each intrinsic function. I like having these separated because if/when we have our ownManageIQ
intrinsic functions they will be nicely separated.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe? methods named with capitals will surely get confusing. If they are class methods then, technicall
::
will call those too, which works, but would still be weird. Reminds me of the Vmdb::BUILD method we have.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like going the
send
route, aka:def StringToJson
(ordef intrinsicStringToJson
if we need to start with a lowercase). Alternatively acase
statement may not be the most ruby thing, but that tends to be the defacto implementation for this type of construct across all programming languages (both target language and complier/interpreter language).All intrinsics are in the same namespace:
States.*
. I think hardcoding this rather than a generic "look for a period" works best, and we can hardcode any additional namespaces in the future. Lets also avoid regular expressions and useindexOf(")")
kind of stuff. It is way too hard to deal with properly nesting parens and regular expressions. Though this implementation may be ok for a first pass.