Skip to content
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

Choice rule payload validation path #253

Merged
merged 2 commits into from
Aug 8, 2024

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Aug 1, 2024

extracted from #189

Before

If the path referenced a missing value in the input, treat it as a nil.

After

If the path references a missing value in the input, raise an error.

  • introduced presence_check method to check if it is present up front.
  • This also affects Intrinsic methods. You can see the changed spec.
  • This respects IsPresent: false and IsPresent: true

@kbrock kbrock added the enhancement New feature or request label Aug 1, 2024
expect(described_class.new("$$.foo").value({})).to be_nil
expect { described_class.new("$$.foo").value({}, {"foo" => "bar"}) }.to raise_error(Floe::PathError, "Path [$$.foo] references an invalid value")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the essence of the issue

@kbrock kbrock mentioned this pull request Aug 1, 2024
11 tasks
@kbrock kbrock force-pushed the choice_rule_payload_validation-path branch from 580ca92 to dceeab9 Compare August 2, 2024 19:49
@kbrock
Copy link
Member Author

kbrock commented Aug 2, 2024

update:

  • removed spec updates/refactor (I'll put in another PR)

Before:

If the path referenced a missing value in the input, treat it as a nil.

After:

If the path references a missing value in the input, raise an error.
This makes it necessary to catch errors and ensure that the context
is marked appropriately.

Since a path lookup is so stringent, introduce the presence_check.
@kbrock kbrock force-pushed the choice_rule_payload_validation-path branch from dceeab9 to 22c08f9 Compare August 2, 2024 21:05
@miq-bot
Copy link
Member

miq-bot commented Aug 2, 2024

Checked commits kbrock/floe@29f923a~...22c08f9 with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
10 files checked, 0 offenses detected
Everything looks fine. 👍

@kbrock
Copy link
Member Author

kbrock commented Aug 2, 2024

update:

  • removed changes to is_present? -- it is not useful now, will delete after the dust settles.

@kbrock kbrock changed the title Choice rule payload validation path [WIP] Choice rule payload validation path Aug 8, 2024
@kbrock kbrock added the wip label Aug 8, 2024
@kbrock
Copy link
Member Author

kbrock commented Aug 8, 2024

WIP: moving stuff around. Want to get other stuff merged before this.
There is no outstanding work for this item.

@kbrock kbrock changed the title [WIP] Choice rule payload validation path Choice rule payload validation path Aug 8, 2024
@kbrock kbrock removed the wip label Aug 8, 2024
@kbrock
Copy link
Member Author

kbrock commented Aug 8, 2024

WIP: oops. thought this was something else.
This is good to go

Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM

@agrare agrare merged commit 6f47120 into ManageIQ:master Aug 8, 2024
4 of 5 checks passed
@kbrock kbrock deleted the choice_rule_payload_validation-path branch August 8, 2024 15:46
agrare added a commit that referenced this pull request Aug 12, 2024
Added
- Choice rule payload validation path (#253)
- Intrinsics JsonToString and StringToJson (#256)
- Add States.Format intrinsic function (#258)
- Intrinsics States.JsonMerge (#255)
- Enable support for Hashes in States.Hash (#260)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants