Question about filters

[jnylen]

Question about filters

[jnylen]

I'm so stupid. Thanks for that.

[mwpastore]

Great! I'll push out a conservative patchlevel release and a more aggressive minor release over the weekend. Thank you so much for helping me troubleshoot this and get it figured out.

[mwpastore]

No need to apologize! I appreciate your interest in Sinja and our chats have been enlightening. Maybe we'd be better off discussing some of the more informal questions on Gitter, but it's not really a big deal to me.

show_many does not support filtering (by any fields other than id), sorting, or paging, and show_many is automatically used instead of index if a filter on id is detected. The logic behind this is two-fold: 1. coalescing of find requests is usually something that happens automatically by the client (i.e. as in the case of Ember Data), so there's no opportunity to further manipulate the collection, and 2. if you know the IDs of the records you want, and the order in which you want them, there's no real sense in further manipulating the collection.

If you want to filter the collection returned by index by any number of other fields, that's totally logical and allowed. All you need to do is define a filter helper that takes the collection returned by index and the hash of query parameters representing the filter and do the appropriate thing for your ORM. The filter_by action helper option comes into play only if you want to restrict the filterable fields.

Here's a simple example that works with Sequel (and in fact this is almost exactly what Sinja::Sequel::Core does):

helpers do
  def filter(collection, fields)
    collection.where(fields) # 2. add a where clause to the dataset

    # ?filter[date]=2017-09-24&filter[otherfilter][]=1&filter[otherfilter][]=2 =>
    # collection.where(:date=>'2017-09-24', :otherfilter=>[1, 2])
  end

  def finalize(collection)
    collection.all # 3. turn the dataset into an array of models
  end
end

index filter_by: [:date, :otherfilter] do
  Model # 1. return the collection as a dataset, not an array of models
end

[jnylen]

Is it a way to require those two fields, like raise an error?

I guess the halt command works in filter?

[mwpastore]

There's no built-in way to require parameters, but yeah, halt works! If you're already using Sinja::Sequel::Core, add this to your resource controller:

(This is wrong, see below for corrected version.)

helpers do
  def filter(_, fields)
    unless fields.key?(:date) && fields.key?(:otherfilter)
      halt 400, 'You must provide date and otherfilter filters' 
    end

    super
  end
end

index filter_by: [:date, :otherfilter] do
  # ..
end

[jnylen]

Sadly i'm hitting Invalid 'filter' query parameter(s) with that somehow.

And /call is still being able to be hit

[mwpastore]

Hrrm, what's the full URL and what does the index call site look like?

[jnylen]

The full url is http://localhost:2300/v3/batches?filter[date]=2017-09-24&filter[channel_id][]=1 (channel_id = otherfield)

Whole resource

    resource :batch do
      helpers do
        def filter(_, fields)
          unless fields.key?(:date) && fields.key?(:channel_id)
            halt 400, 'You must provide date and otherfilter filters'
          end

          super
        end

        def find(id)
          Batch[id.to_i]
        end
      end

      show

      index(filter_by: [:date, :channel_id]) do
        Batch
      end
    end

The filter work on other calls but not on that specific one, even if I remove the special filter requirement.

[mwpastore]

That certainly looks like a bug. Let me try to get a local repro and I'll work on a fix. Thanks!

[jnylen]

Alrighty,
@mwpastore can you also look into a way to mark filters as integer on filters?

Let's say I have a movie model with a imdb_id as integer (instead of tt), the default provider then does a search for the thing as a string instead of a integer.

[mwpastore]

RE: integer filters, here's the easiest way to do that, currently. In your resource controller:

helpers do
  def filter(_, fields)
    fields[:imdb_id] = fields[:imdb_id].to_i

    super
  end
end

[mwpastore]

Slight correction to the above. To require filter fields, you'll want to put the check in before_index, not filter. The reason is because filter isn't invoked if the filter query parameter isn't present. That would look something like this in your resource controller:

helpers do
  def before_index
    unless params[:filter].key?(:date) && params[:filter].key?(:channel_id)
      halt 400, 'You must provide date and otherfilter filters'
    end
  end
end

[jnylen]

Okey, now it complains if those filters isn't there, thanks.

It still complains that the filter query is invalid if those two are provided.

I also understand that channel_ids turns it into an array but using fitler[channel_id][] doesnt turn it into an array.

Dump of invalid filter query:

{"date"=>"lbalbalba", "channel_id"=>"1"}
E, [2017-09-26T13:55:43.221353 #10359] ERROR -- sinja: {:id=>"e0945391-640c-40f3-881a-1f4143714626", :title=>"Bad Request Error", :detail=>"Invalid `filter' query parameter(s)", :status=>"400"}

[jnylen]

Seems to be an issue with my

index(filter_by: [:date, :channel_id]) do
  Batch
end

If I remove filter_by it works.

And doing .to_i on some filters (in this case channel_id) doesn't seem to go well with the .where() as it seems it complains you can't go from string to integer.

2017-09-26 14:26:04 - Sequel::DatabaseError - PG::InvalidTextRepresentation: ERROR:  invalid input syntax for integer: "channel_id"
LINE 1: ...ROM "batches" WHERE (('date' = '2017-09-24') AND ('channel_i...

if I run the model manually with the data with channel_id as a integer it works.

[mwpastore]

These are all good findings that I want to fix! Bear with me!

[mwpastore]

@jnylen Could you share with me the full SQL log for the bad query mentioned in your note from last week? The relevant part (about channel_id) is cut off.

[jnylen]

Hi @mwpastore,
So without .to_i:

INFO -  (0.000732s) SELECT * FROM "batches" WHERE (('date' = '2017-09-26') AND ('channel_id' = '38'))

With .to_i:

E, [2017-10-03T23:43:05.458291 #31350] ERROR -- : PG::InvalidTextRepresentation: ERROR:  invalid input syntax for integer: "channel_id"
LINE 1: ...ROM "batches" WHERE (('date' = '2017-09-26') AND ('channel_i...
                                                             ^: SELECT * FROM "batches" WHERE (('date' = '2017-09-26') AND ('channel_id' = 38))

Doing it manually in padrino console works. SQL:

INFO -  (0.000833s) SELECT * FROM "batches" WHERE (("date" = '2017-09-26') AND ("channel_id" = 38))

[mwpastore]

@jnylen The filter helper I suggested above is working fine.

The problem is that somewhere along the line the filter keys are being transformed from symbols to strings before being passed to Sequel, so Postgres is literally trying to compare the string 'channel_id' to the integer 38, and that's an invalid comparison. You can see in the runtime output that the fields are being single-quoted (the wrong behavior), which is how Postgres escapes a string, and in the Padrino console output that the fields are being double-quoted (the correct behavior), which is how Postgres escapes an identifier.

This is also why you're getting the error about invalid filter query parameters: Sinja is whitelisting symbols, but seeing strings.

So now the question is: why and where is this happening? I thought you switched to Sinatra::Base for your controllers, are you still using Padrino? What versions of Sinatra and Padrino are present in your gem bundle?

[mwpastore]

I think I need to cut a new release of Sinja with some of the work that I've pushed up over the past couple months. Do me a favor: try running the bleeding edge and seeing if that fixes the problem:

gem 'sinja', git: 'https://github.com/mwpastore/sinja.git', branch: 'master'

[jnylen]

So, @mwpastore.

Using the git repo instead of the one on rubygems works.

INFO -  (0.111916s) SELECT * FROM "batches" WHERE (("date" = '2017-08-26') AND ("channel_id" = 38))

And using [] on non *_ids works aswell.

So everything that failed seems to work on the git repo.

So closing this. ;)