Some Days...

I started today with a (I thought) pretty simple task that I expected to be able to dispense with in an hour or two. The task was to modify param parsing to correctly handle foo=bar&foo=baz. In Rails, that returns {"foo" => "bar"}. In Merb, it returns {"foo" => "baz"}. In Rack, it returns {"foo" => ["bar", "baz"]}. In other words, a crazy state of affairs.

Last night, we agreed to attempt something like the following:

>> q = Rack::Util.parse_query("foo=bar&foo=baz")
=> {"foo" => "baz"}
>> q["foo"]
=> "baz"
>> q.all("foo")
=> ["bar", "baz"]
>> q.each {|key, value| p [key, value] }
["foo", "baz"]
=> nil

The idea was cribbed from WebOb, a popular Python library built for WSGI (the inspiration for Rack). WebOb uses their own MultiDict, and the patch I wrote introduces a MultiHash for Ruby with similar semantics.

Next, I needed to modify the Rack tests because they initially tested the actual resulting Hash from parsing parameters, and since the structure was internally different, I needed to make some changes to the tests to make them pass. Here's a sample line from the diff:

-      should.equal "foo" => "1", "bar" => "2"
+      should.match_hash "foo" => "1", "bar" => "2"

Rack is using test/spec, so the matcher looks like:

class Test::Spec::Should
  def match_hash(hash)
    matches = true
    hash, obj = hash.dup, @object.dup
    while !hash.empty?
      key, value = hash.shift
      assert_equal value, obj.delete(key), "#{value.inspect} expected for key #{key.inspect}"
    end
    assert obj.empty?, "Some additional keys were found: #{obj.inspect}"
  end
end

I'd imagine that it could use some work, but this did the trick for the moment. I also added a new context for the new behavior:

context "when the same value is supplied twice and it does not end in []" do
  before do
    @hash = Rack::Utils.parse_query("foo=bar&foo=quux")
  end
  
  specify "should return the last parsed value when indexed" do
    @hash.should.match_hash "foo" => "quux"
  end
  
  specify "should return all of the values with .all" do
    @hash.all("foo").should.equal ["bar", "quux"]
  end
end

Now that all the tests passed, I moved on to Rails, and then proceeded to spend more hours than I'd like to admit trying to figure out how a seemingly unrelated bug was triggered by this change. As it turned out, I was running the latest version of Rack before the repo was moved, and there was a change to part of the same file (utils.rb) that broke Rails. A few more tweaks later and all tests pass.

The proposal should be hitting the rack mailing list in the next day or so, and I'll be sure to keep everyone up to date on how that's going.