Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot access objects with chained has many associations #89

Open
paulvt opened this issue Dec 18, 2017 · 4 comments
Open

Cannot access objects with chained has many associations #89

paulvt opened this issue Dec 18, 2017 · 4 comments

Comments

@paulvt
Copy link

paulvt commented Dec 18, 2017

It seems that the association only adds the parameter of the parent (and not all ancestors), see lib/spyke/associations/has_many.rb#L7. As a result, when going through two relations, the URL cannot be constructed any more because there is a parameter missing. For example:

class Recipe < Spyke::Base
  has_many :ingredients
end

class Ingredient < Spyke::Base
  uri 'recipes/:recipe_id/ingredients/(:id)'
  has_many :alternatives, class_name: 'IngredientAlternative',
                          uri: 'recipes/:recipe_id/ingredients/:ingredient_id/alternatives'
end

class IngredientAlternative < Spyke::Base
  uri 'recipes/:recipe_id/ingredients/:ingredient_id/alternatives/(:id)' 
end

recipe = Recipe.all.first
ingredient = recipes.ingredients.first
alternative = ingredient.alternatives.first

This yields:

Spyke::InvalidPathError: Missing required variables: recipe_id, in recipes/:recipe_id/ingredients/:ingredient_id/alternatives. Mark optional variables with parens eg: (:param)

I am not too familiar with the code base yet, so I haven't come up with a pull request. A solution might be that HasMany#initialize adds all :<something>_id key/values to params that it finds in the URI, not just the foreign_key that is used in the default URI?

@balvig
Copy link
Owner

balvig commented Dec 19, 2017

Would be great to also see the JSON responses for this, seems like it might be related to #90 🙇 🙏

@paulvt
Copy link
Author

paulvt commented Dec 20, 2017

I have used the following slightly modified class definitions in https://github.com/balvig/spyke/blob/master/test/support/fixtures.rb:

class Ingredient < Spyke::Base
  uri 'recipes/:recipe_id/ingredients/(:id)'
  has_many :alternatives, class_name: "IngredientAlternative",
                          uri: 'recipes/:recipe_id/ingredients/:ingredient_id/alternatives'
end

class IngredientAlternative < Spyke::Base
  uri 'recipes/:recipe_id/ingredients/:ingredient_id/alternatives/(:id)'
end

and added the following test:

    def test_chained_associations
      json = { result: { id: 1, groups: [{ ingredients: [{ id: 2, recipe_id: 1, name: 'Fish' }] }] } }
      stub_request(:get, 'http://sushi.com/recipes/1').to_return_json(json)
      json = { result: [{ id: 3, name: "Tofu" }] }
      stub_request(:get, 'http://sushi.com/recipes/1/ingredients/2/alternatives').to_return_json(json)

      recipe = Recipe.find(1)
      assert_equal 1, recipe.id
      ingredient = recipe.ingredients.first
      assert_equal recipe.id, ingredient.recipe_id
      assert_equal 2, ingredient.id

      alternative = ingredient.alternatives.first
      assert equal 3, alternative.id
    end

which fails with the exception/error described in the original bug report.
I have inspected the path object used in the has_many relation alternatives and it is:

#<Spyke::Path:0x00000004ac82e0
  @pattern="recipes/:recipe_id/ingredients/:ingredient_id/alternatives",
  @params={:ingredient_id=>2},
  @uri_template=#<Addressable::Template:0x255dd98 PATTERN:recipes/{recipe_id}/ingredients/{ingredient_id}/alternatives>,
  @variables=[:recipe_id, :ingredient_id]>

The ingredient_id is in there because of line 7 in has_many.rb, but of course it didn't include the recipe_id from the has many relation ingredients through which we arrived at the parent.

@balvig
Copy link
Owner

balvig commented Dec 22, 2017

Thank you @paulvt!

Seems this is related to Spyke being designed on the assumption of only having "one level of nesting" for associations (ie /recipes/:recipe_id/something will work but /recipes/:recipe_id/:user_id/something won't!)

Not sure of the best way to fix this yet, it maybe needs a rethink about how associations piece together the URI for sending a request 🤔

@paulvt
Copy link
Author

paulvt commented Dec 22, 2017

Yes, I have been thinking about this. I can think of two ways...

  1. A way to access information of the parent, that received information from its parent, accumulating the IDs to be able to build the path. I.e. the parent remembers the params needed to "get there" and can offer its information to associations.

  2. Or, maybe more straight forwardly, scan the URI for used ID variables and add those to the Path object of the relation.
    The latter in fact already kind of happens, just in a hardcoded way:

...
    class HasMany < Association
      def initialize(*args)
        super
        @options.reverse_merge!(uri: "#{parent_path}/:#{foreign_key}/#{@name}/(:#{primary_key})")
        @params[foreign_key] = parent.id
      end
      ...
        def parent_path
          parent.class.model_name.element.pluralize
        end
   end

If you look at the above code, for the default URI that would have been used, ingredients/:ingredient_id/alternatives/:id the param :ingredient_id is added by default for the default URI, taken from the parent.

In this case, would just would like to tell the relation that first, I want to use a different URI namely recipes/:recipe_id/ingredients/:ingredient_id/alternatives/:id, and second to need use the two params :ingredient_id and :recipe_id available on the object.

The latter solution does require that the API provides both IDs though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants