Fixing an edge case when using objects as constraints

[simonc]

Hello 😊

Summary

This PR fixes an issue when the following situation occurs.
If you define a class like this:

class MyConstraint
  def call(*args)
    # for some reason this is defined
  end

  def matches?(*args)
    # checking the args
  end
end

and try to use it as a constraint:

get "/", to: "home#show", constraints: MyConstraint.new

if its matches? method returns false there will be an error because the mapper will ask for the constraint arity, thinking it is a proc, lambda or method.

The new code checks for the presence of the arity method on the constraint, calling it only if present, preventing the error while keeping the basic behavior.

[Edouard-chin]

Isn't the problem going to be the same if the call method of your class except a determined number of argument? (the mapper will always pass 2 arguments to the method).

Maybe it would be be better to check if the object is a proc instead of looking if it responds to call. This is also inline with what the documentation state (either pass a proc, either a class that responds to matches?)
https://github.com/rails/rails/pull/34004/files#diff-46876897330259072a679792c5f45f99L41

[rafaelfranca]

Why did you changed the way the private method is defined?

[simonc]

@rafaelfranca I changed it because it was like that in other places of the same file 😊

[simonc]

Lines 187 and 192 for instance.

[rafaelfranca]

Better to leave as it is. Line 255 is using the previous code.

[simonc]

@Edouard-chin I thought about it but didn't want to make too big of a change. I suppose I could simply ask for the arity of the call method instead of asking the object directly 🤔

Something like

def constraint_args(constraint, request)
  arity =
    if constraint.respond_to?(:arity)
      constraint.arity
    else
      constraint.method(:call).arity
    end

  return [] if arity < 1
  return [request] if arity == 1
  [request.path_parameters, request]
end

I guess this doesn't cover the case where the call method is unrelated and expects 3+ arguments but I think we're getting to the edge of an edge-case 😅

[rafaelfranca]

If the call is unrelated it will be called and it should respond to the number of arguments passed that constraint_args returns, so I don't think a constraint can have a unrelated call and expect 3+ arguments. I think we We should check the arty of call.

[simonc]

@rafaelfranca alright, I'll update the code to the one in my previous comment. Any convention regarding the if alignment or can I copy/paste my comment?

edit:

Ok apparently the convention is the following so I will do that

arity = if constraint.respond_to?(:arity)
  constraint.arity
else
  constraint.method(:call).arity
end

[rafaelfranca]

Merged in 6a73faa