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

add UriFilter to additional filters #118

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

estraph
Copy link

@estraph estraph commented Oct 22, 2017

This PR adds a filter for URI objects as inputs to commands. Ensures the URI is not empty and that it parses.

Usage example, taken from a command I wrote to wrap a HTTP API:

required do
  uri :endpoint, scheme: :https
  string :api_key, default: nil
end

Tests pass locally:

Raphs-MacBook-Pro:mutations raph$ git status
On branch raph/uri-filter
nothing to commit, working directory clean
Raphs-MacBook-Pro:mutations raph$ rake
Run options: --seed 36168

# Running tests:

...........................................................................................................................................................................................................................................

Finished tests in 0.069457s, 3383.3883 tests/s, 7285.0829 assertions/s.

235 tests, 506 assertions, 0 failures, 0 errors, 0 skips

This PR adds a filter for URI objects as inputs to commands.
begin
data = URI.parse(data)
rescue StandardError => e
return [nil, e.message]
Copy link

Choose a reason for hiding this comment

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

@estraph The filter operation always starts from the HashFilter and from there it will filter the nested input. The results of the nested filter will always flow back to HashFilter.

What I can see is all basic filter like StringFilter, FloatFilter and etc always return Array<Object, Symbol> in #filter. Are you sure returning e.message here will be handled gracefully by the error handler?

I think you might see the issue if you write a rspec with a Mutation::Commands that will trigger error on this filter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants