Where with the primary key results in a find

[johan-smits]

When you do a where query with the primary key in it uses does a find.

It should be possible to do for example: Recipe.where(id: [1,2], title: 'foo') but now it results in: http://sushi.com/recipes/1,2?title=foo and it should be: http://sushi.com/recipes?id[]=1&id[]=2&title=foo

#test/orm_test.rb
def test_where
  endpoint = stub_request(:get, 'http://sushi.com/recipes?id=1').to_return_json(result: [{ id: 1, title: 'Sushi' }])

  recipe = Recipe.where(id: 1).first

  assert recipe

  assert_equal 1, recipe.id
  assert_equal 'Sushi', recipe.title
  assert_requested endpoint
end

[balvig]

@johan-smits Ah, I think in order for that to happen, you will need to drop down to a custom request (some examples found here).

The intended behavior of Spyke is to “fill out” placeholders defined in the URI (ie :id, :user_id) etc with the parameters you pass into where, so for example:

Recipe.with(“users/:user_id/recipes”).where(user_id: 3) # => /users/3/recipes

For your situation I guess you could do something like:

Recipe.with(“recipes”).where(id: [1,2])

Hope that clarifies things?

[paulvt]

IMO, while his makes sense if you look at the implementation (also because #find goes through #where to build the relation)., this is not very ActiveRecord-like and surprising to the user.
In our case, we're building a library (not an app) previously based on Her and now Spyke. So this will be hard to explain given that we abstract over URIs, so the user will know nothing about this.

[johan-smits]

@balvig I understand the examples but it is counter intuitive. When for example the recipes endpoint changes from its location I have to modify it from the model and all locations that I use this in the code.
Also when a endpoint had more placeholder defined you have to keep that in mind when programming.
I use this gem in a rails project and until now I could write the same queries and code not knowing how the models internals and URIs are.

[balvig]

@paulvt and @johan-smits I probably don't fully understand the problems you're running into yet 🤔

When for example the recipes endpoint changes from its location I have to modify it from the model and all locations that I use this in the code. Also when a endpoint had more placeholder defined you have to keep that in mind when programming.

Ah really sorry, but am having a hard time picturing this actual problem! 🙇
Maybe if you can share some more code showing the friction this causes I could get a better idea? 🙏

Gut feeling is that if we for example make where ignore placeholders completely (not sure if that is the proposal), that also becomes a (different) problem 🤔 For example:

class Recipe
  uri "users/:user_id/recipes/:id"
end

Recipe.where(user_id: 4).find(6) #=> /users/:user_id/6?user_id=4

[paulvt]

Yes, for chained associations this becomes more of a problem. I think that Her has no issue with this because it has a separate notion of collection and resource URI. Obviously, then,#where always uses the collection URI.

class User
  has_many :recipes, uri: "users/:user_id/recipes"
end

class Recipie
  uri "users/:user_id/recipes/(:id)"
end

user = User.find(4)
user.recipes.find(6)      # Use resource URI users/:user_id/recipes/:id
user.recipes.where(id: 6) # Use collection URI users/:user_id/recipes, still fill in :user_id

However, for Spyke, where uses #where to do a #find, so how would it know which URI type to use given there is such a notion?

[paulvt]

The problem is mainly for the users of our library, who would expect to be able to do:

my_recipes = Recipes.where(id: [1, 2, 3, 4]).to_a

not known anything about the underlying URIs of the Spyke classes.

[balvig]

Ah, maybe I made things more confusing by using something resembling an association as an example 🙇

In your example above, using uri “users/:user_id/recipes/(:id)”, I wonder what should happen if you do a straight Recipe.find?

I guess as I developer I would also expect Recipe.where(user_id :5) etc to work and be surprised when it didn’t?

I suppose this would force the developer to always have to go through a User to get a certain Recipe?

Maybe it helps to think of something that’s not an association, I would be surprised if this didn’t work for example:

class Recipe
  uri “:cuisine/recipes/(:id)”
end

Recipe.where(cuisine: “french”).find(5) #=> /french/recipes/5

I’m not completely understanding how using where to do a find is related to the issue, I guess it’s more about deciding when/when not to respect placeholders? That’s where I’m finding it hard to come up with a rule that doesn’t make a compromise somewhere! 😅

Do you have a proposal on how you imagine this should work? I think there’s always going to be some cases where we can’t 100% emulate ActiveRecord (ie just the fact that some endpoints might not exist etc!)

Without having tried it, I think for your specific use case, I would consider adding something like:

def self.where(params = {})
  if params[:id].is_a?(Array)
    with(“recipes”).where(params)
  else
    super
  end
end

[paulvt]

The straight Recipe.find call when using an URI that has other IDs, should not work of course. This is already the case, Spyke reports missing parameters.

We are looking for an API similar to AR, like documenter here: https://apidock.com/rails/ActiveRecord/FinderMethods/find.

I've pushed a branch that strips the optional primary key from "standard" URI (thus generating a "collection URI"), so that the parameter is not filled in (see also the test).
The relevant commit of my multi-find can be found here: LeftClickBV@e2781b6.

[johan-smits]

@paulvt your branch works for me.

[johan-smits]

@balvig did you had the chance to review the branch of @paulvt ?

[balvig]

@johan-smits thanks, I did. Initial take was it it felt a little more...invasive? than I feel comfortable with 🙇 so I wanted to think about it a bit.

Thank you for the clear spec though and the implementation proposal, but I still don't feel 100% sure if this is something that should live as a permanent part of Spyke, as it seems there are many opinions on how retrieving multiple IDs from an api should work.

I guess your proposal is:

GET http://sushi.com/recipes?id[]=1&id[]=2

But I've also seen APIs that handle it this way:

GET http://sushi.com/recipes?ids=1,2

and for example Her handles it this way:

GET http://sushi.com/recipes/1
GET http://sushi.com/recipes/2

...which kinda makes me suspect it's a decision that needs to be made on a per-API basis...? 🤔

[paulvt]

Yes, I agree. The changes in my branch for Spyke::ORM.find seem fine to me; it just makes find more similar to AR. The other part, though, is indeed quite API-specific and invasive. The problem here is how to give the library user a way to specify/override how this is handled.
I do dislike my change on Spyke::HTTP.scoped_request but didn't know any other way.

Also, note that the changes lean on Spyke's current behaviour for multi-valued queries, so that id[]=1,id[]=2… is something that Spyke did, not I ;).

[johan-smits]

It makes the 1,2 only when it is the primary key. If not it uses the id[].. notation. This is not consistent.

[balvig]

It makes the 1,2 only when it is the primary key. If not it uses the id[].. notation. This is not consistent.

Ah, to clarify, it's not really about being the primary key, the behavior is based on whether the parameter is a variable in the URI or not, so for example any of these (not just :id) would take the passed in where conditions and make them part of URI:

/categories/:category_id/recipe/:id/:action

In that sense I believe it's actually pretty consistent?

Again, I believe you can use Spyke as a starting point upon which you add the behavior unique to your API and any methods your users require.

Therefore, if you have a requirement that find needs to accept an array, and that when it does it is to leave out the :id param from the URL and instead send it as query params, it would be perfectly acceptable for you to extend the method in your Base class that the other parts of the API inherits from, ie:

module Api
  class Base < Spyke::Base
    def self.find(ids*)
      # ... define the behavior you want
    end
  end

  class Post < Base
  end 
end

[paulvt]

The problem I have in this case is then that it not only requires and override of Base.find but also Skype::HTTP.scoped_request. If there is any way to do this more cleanly, I'd like to follow that and try it in our setup.