RestClient::BadRequest in latest commit

[alexanderedge]

Hi there,

Commit 2c5b768 seems to have broken something, since I am getting RestClient::BadRequest exception when listing a resource.

Additional Information

• Using CouchRest 1.2.0
• Works fine in 1cf59de

[samlown]

Hi Alex. Could it be that you are modifying a view object by setting nil values? Would be great if you could provide some sample code, such as the view definition and example of the view you are building.

[alexanderedge]

Hi Sam,

No, I'm not modifying a view object, and it seems to happen when trying to display all items in any class in my Rails application. Here is an example view definition:

design do
    view :by_name
end

What might be pertinent is that my CouchDB connection has a username and password.

[samlown]

There is nothing out of the ordinary there Alex. There must be something different in your environment that is not covered by our test cases. Perhaps you could paste the entire model except the bits that do not involve CouchDB?

I'm guessing you are doing something like:

SomeClass.by_name.all

and get the error?

[alexanderedge]

Okay, so I've narrowed this down somewhat.

In my controller, I have the following:

@people = People.by_name.skip(params[:skip]).limit(params[:limit])

This is the corresponding call with 1cf59de:

GET /myapp_people_development/_design/People/_view/by_name?include_docs=true&reduce=false 200

And this is the corresponding call with 2c5b768:

GET /myapp_people_development/_design/People/_view/by_name?skip=&limit=&include_docs=true&reduce=false 400

As you can see, the skip and limit parameters are included even though their values are nil.

[alexanderedge]

Which, now I read, is the point of 2c5b768, but I'm not sure that I follow the rationale for it.

[samlown]

That makes more sense now :-) The reason for the change in 2c5b768 is to make sure we always send something to the CouchDB server to avoid the .key(nil) issue that would otherwise always return a document. IMHO, setting the skip and limit options comes under the same basket; they shouldn't really be set if there is no value. If for example would always set a default value:

People.by_name.skip(params[:skip] ||= 0).limit(params[:limit] ||= 30)

You might also want to consider using the .page(params[:page]).per(params[:limit]) options. These will automatically work around nil values and provide decent support for the kaminari gem.

[alexanderedge]

Thanks a lot, Sam, I'll make the change now.

Also, I'm new to Rails so your code-style tips are much appreciated.

[alexanderedge]

It looks like Kaminari also comes a cropper with this commit, with the skip query added but not used when not enough objects can fill a page:

GET /myapp_people_development/_design/People/_view/by_name?limit=0&skip= 400

[samlown]

I'm not sure I understand... clearly the limit there is at 0, which is wrong. Could you provide the view methods you're using with example data?