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

Bug: include_data loads association #1707

Closed
richmolj opened this issue Apr 25, 2016 · 2 comments
Closed

Bug: include_data loads association #1707

richmolj opened this issue Apr 25, 2016 · 2 comments

Comments

@richmolj
Copy link
Contributor

Expected behavior vs actual behavior

Given the following serializer:

class PostSerializer < ActiveModel::Serializer
  has_many :comments do
    link :related do
      href "/posts/#{object.id}/comments"
    end
    include_data false
  end
end

This correctly does not include comment relationship data in the response. However, all the comments are still loaded (ie select * from comments where post_id = ?). I would expect this to avoid loading the comments since none of that data is needed in the response.

Steps to reproduce

Any serializer with include_data false, look at the ActiveRecord queries that fire.

Environment

ActiveModelSerializers Version (commit ref if not on tag): master/0165215

Output of ruby -e "puts RUBY_DESCRIPTION": ruby 2.2.3p173 (2015-08-18 revision 51636) [x86_64-darwin14]

OS Type & Version: OSX 10.10.5 (Yosemite)

Integrated application and version (e.g., Rails, Grape, etc): Rails 5 beta 3

@beauby
Copy link
Contributor

beauby commented Apr 25, 2016

You are right, I've had this one on my radar for a while. Basically we would just need to lazily build the associations from the reflections (plus, this would allow us to tackle the optim on "links-only belongs_to").

@groyoh groyoh self-assigned this Apr 29, 2016
groyoh pushed a commit to groyoh/active_model_serializers that referenced this issue Apr 29, 2016
groyoh pushed a commit to groyoh/active_model_serializers that referenced this issue Apr 29, 2016
richmolj pushed a commit to richmolj/active_model_serializers that referenced this issue May 9, 2016
The current implementation does support conditionally sideloading
relationships based on the 'include' URL param. However, omitting the
relationship still loads the relationship (to populate the type/id
'relationships' payload), somewhat defeating the purpose. Instead, this
changes the flow to:

1. If the relationship is included, load it and include it the response.
2. If the relationship is not included but there is a JSONAPI link,
include the link in the response but do not load the relationship or
include data.
3. If the relationship is not in the URL param and there is no link, do
not include this node in the 'relationships' response.

The `current_include_tree` edits in json_api.rb are to pass the current
nested includes. This is to support when multiple entities have the same
relationship, e.g. `/blogs/?include=posts.tags,tags` should include both
blog tags and post tags, but `/blogs/?include=posts.tags` should only
include post tags.

This API is opt-in to support users who always want to load
`relationships` data. To opt-in:

```ruby
class BlogSerializer < ActiveModel::Serializer
  associations_via_include_param(true) # opt-in to this pattern

  has_many :tags
  has_many :posts do
    link :self, '//example.com/blogs/relationships/posts'
  end
end
```

JSONAPI include parameters (http://jsonapi.org/format/#fetching-includes).

Fixes rails-api#1707
Fixes rails-api#1555
richmolj pushed a commit to richmolj/active_model_serializers that referenced this issue May 9, 2016
The current implementation does support conditionally sideloading
relationships based on the 'include' URL param. However, omitting the
relationship still loads the relationship (to populate the type/id
'relationships' payload), somewhat defeating the purpose. Instead, this
changes the flow to:

1. If the relationship is included, load it and include it the response.
2. If the relationship is not included but there is a JSONAPI link,
include the link in the response but do not load the relationship or
include data.
3. If the relationship is not in the URL param and there is no link, do
not include this node in the 'relationships' response.

The `current_include_tree` edits in json_api.rb are to pass the current
nested includes. This is to support when multiple entities have the same
relationship, e.g. `/blogs/?include=posts.tags,tags` should include both
blog tags and post tags, but `/blogs/?include=posts.tags` should only
include post tags.

This API is opt-in to support users who always want to load
`relationships` data. To opt-in:

```ruby
class BlogSerializer < ActiveModel::Serializer
  associations_via_include_param(true) # opt-in to this pattern

  has_many :tags
  has_many :posts do
    link :self, '//example.com/blogs/relationships/posts'
  end
end
```

JSONAPI include parameters (http://jsonapi.org/format/#fetching-includes).

Fixes rails-api#1707
Fixes rails-api#1555
richmolj pushed a commit to richmolj/active_model_serializers that referenced this issue May 9, 2016
The current implementation does support conditionally sideloading
relationships based on the 'include' URL param. However, omitting the
relationship still loads the relationship (to populate the type/id
'relationships' payload), somewhat defeating the purpose. Instead, this
changes the flow to:

1. If the relationship is included, load it and include it the response.
2. If the relationship is not included but there is a JSONAPI link,
include the link in the response but do not load the relationship or
include data.
3. If the relationship is not in the URL param and there is no link, do
not include this node in the 'relationships' response.

The `current_include_tree` edits in json_api.rb are to pass the current
nested includes. This is to support when multiple entities have the same
relationship, e.g. `/blogs/?include=posts.tags,tags` should include both
blog tags and post tags, but `/blogs/?include=posts.tags` should only
include post tags.

This API is opt-in to support users who always want to load
`relationships` data. To opt-in:

```ruby
class BlogSerializer < ActiveModel::Serializer
  associations_via_include_param(true) # opt-in to this pattern

  has_many :tags
  has_many :posts do
    link :self, '//example.com/blogs/relationships/posts'
  end
end
```

JSONAPI include parameters (http://jsonapi.org/format/#fetching-includes).

Fixes rails-api#1707
Fixes rails-api#1555
richmolj pushed a commit to richmolj/active_model_serializers that referenced this issue May 9, 2016
The current implementation does support conditionally sideloading
relationships based on the 'include' URL param. However, omitting the
relationship still loads the relationship (to populate the type/id
'relationships' payload), somewhat defeating the purpose. Instead, this
changes the flow to:

1. If the relationship is included, load it and include it the response.
2. If the relationship is not included but there is a JSONAPI link,
include the link in the response but do not load the relationship or
include data.
3. If the relationship is not in the URL param and there is no link, do
not include this node in the 'relationships' response.

The `current_include_tree` edits in json_api.rb are to pass the current
nested includes. This is to support when multiple entities have the same
relationship, e.g. `/blogs/?include=posts.tags,tags` should include both
blog tags and post tags, but `/blogs/?include=posts.tags` should only
include post tags.

This API is opt-in to support users who always want to load
`relationships` data. To opt-in:

```ruby
class BlogSerializer < ActiveModel::Serializer
  associations_via_include_param(true) # opt-in to this pattern

  has_many :tags
  has_many :posts do
    link :self, '//example.com/blogs/relationships/posts'
  end
end
```

JSONAPI include parameters (http://jsonapi.org/format/#fetching-includes).

Fixes rails-api#1707
Fixes rails-api#1555
richmolj pushed a commit to richmolj/active_model_serializers that referenced this issue May 12, 2016
The current implementation does support conditionally sideloading
relationships based on the 'include' URL param. However, omitting the
relationship still loads the relationship (to populate the type/id
'relationships' payload), somewhat defeating the purpose. Instead, this
changes the flow to:

1. If the relationship is included, load it and include it the response.
2. If the relationship is not included but there is a JSONAPI link,
include the link in the response but do not load the relationship or
include data.
3. If the relationship is not in the URL param and there is no link, do
not include this node in the 'relationships' response.

The `current_include_tree` edits in json_api.rb are to pass the current
nested includes. This is to support when multiple entities have the same
relationship, e.g. `/blogs/?include=posts.tags,tags` should include both
blog tags and post tags, but `/blogs/?include=posts.tags` should only
include post tags.

This API is opt-in to support users who always want to load
`relationships` data. To opt-in:

```ruby
class BlogSerializer < ActiveModel::Serializer
  associations_via_include_param(true) # opt-in to this pattern

  has_many :tags
  has_many :posts do
    link :self, '//example.com/blogs/relationships/posts'
  end
end
```

JSONAPI include parameters (http://jsonapi.org/format/#fetching-includes).

Fixes rails-api#1707
Fixes rails-api#1555
richmolj pushed a commit to richmolj/active_model_serializers that referenced this issue May 23, 2016
The current implementation does support conditionally sideloading
relationships based on the 'include' URL param. However, omitting the
relationship still loads the relationship (to populate the type/id
'relationships' payload), somewhat defeating the purpose. Instead, this
changes the flow to:

1. If the relationship is included, load it and include it the response.
2. If the relationship is not included but there is a JSONAPI link,
include the link in the response but do not load the relationship or
include data.
3. If the relationship is not in the URL param and there is no link, do
not include this node in the 'relationships' response.

The `current_include_tree` edits in json_api.rb are to pass the current
nested includes. This is to support when multiple entities have the same
relationship, e.g. `/blogs/?include=posts.tags,tags` should include both
blog tags and post tags, but `/blogs/?include=posts.tags` should only
include post tags.

This API is opt-in to support users who always want to load
`relationships` data. To opt-in:

```ruby
class BlogSerializer < ActiveModel::Serializer
  associations_via_include_param(true) # opt-in to this pattern

  has_many :tags
  has_many :posts do
    link :self, '//example.com/blogs/relationships/posts'
  end
end
```

JSONAPI include parameters (http://jsonapi.org/format/#fetching-includes).

Fixes rails-api#1707
Fixes rails-api#1555
groyoh pushed a commit to groyoh/active_model_serializers that referenced this issue May 24, 2016
@NullVoxPopuli
Copy link
Contributor

Closed by #1710

@NullVoxPopuli NullVoxPopuli reopened this May 25, 2016
richmolj pushed a commit to richmolj/active_model_serializers that referenced this issue May 27, 2016
The current implementation does support conditionally sideloading
relationships based on the 'include' URL param. However, omitting the
relationship still loads the relationship (to populate the type/id
'relationships' payload), somewhat defeating the purpose. Instead, this
changes the flow to:

1. If the relationship is included, load it and include it the response.
2. If the relationship is not included but there is a JSONAPI link,
include the link in the response but do not load the relationship or
include data.
3. If the relationship is not in the URL param and there is no link, do
not include this node in the 'relationships' response.

The `current_include_tree` edits in json_api.rb are to pass the current
nested includes. This is to support when multiple entities have the same
relationship, e.g. `/blogs/?include=posts.tags,tags` should include both
blog tags and post tags, but `/blogs/?include=posts.tags` should only
include post tags.

This API is opt-in to support users who always want to load
`relationships` data. To opt-in:

```ruby
class BlogSerializer < ActiveModel::Serializer
  associations_via_include_param(true) # opt-in to this pattern

  has_many :tags
  has_many :posts do
    link :self, '//example.com/blogs/relationships/posts'
  end
end
```

JSONAPI include parameters (http://jsonapi.org/format/#fetching-includes).

Fixes rails-api#1707
Fixes rails-api#1555
richmolj pushed a commit to richmolj/active_model_serializers that referenced this issue May 28, 2016
The current implementation does support conditionally sideloading
relationships based on the 'include' URL param. However, omitting the
relationship still loads the relationship (to populate the type/id
'relationships' payload), somewhat defeating the purpose. Instead, this
changes the flow to:

1. If the relationship is included, load it and include it the response.
2. If the relationship is not included but there is a JSONAPI link,
include the link in the response but do not load the relationship or
include data.
3. If the relationship is not in the URL param and there is no link, do
not include this node in the 'relationships' response.

The `current_include_tree` edits in json_api.rb are to pass the current
nested includes. This is to support when multiple entities have the same
relationship, e.g. `/blogs/?include=posts.tags,tags` should include both
blog tags and post tags, but `/blogs/?include=posts.tags` should only
include post tags.

This API is opt-in to support users who always want to load
`relationships` data. To opt-in:

```ruby
class BlogSerializer < ActiveModel::Serializer
  associations_via_include_param(true) # opt-in to this pattern

  has_many :tags
  has_many :posts do
    link :self, '//example.com/blogs/relationships/posts'
  end
end
```

JSONAPI include parameters (http://jsonapi.org/format/#fetching-includes).

Fixes rails-api#1707
Fixes rails-api#1555
richmolj pushed a commit to richmolj/active_model_serializers that referenced this issue Jun 1, 2016
The current implementation does support conditionally sideloading
relationships based on the 'include' URL param. However, omitting the
relationship still loads the relationship (to populate the type/id
'relationships' payload), somewhat defeating the purpose. Instead, this
changes the flow to:

1. If the relationship is included, load it and include it the response.
2. If the relationship is not included but there is a JSONAPI link,
include the link in the response but do not load the relationship or
include data.
3. If the relationship is not in the URL param and there is no link, do
not include this node in the 'relationships' response.

The `current_include_tree` edits in json_api.rb are to pass the current
nested includes. This is to support when multiple entities have the same
relationship, e.g. `/blogs/?include=posts.tags,tags` should include both
blog tags and post tags, but `/blogs/?include=posts.tags` should only
include post tags.

This API is opt-in to support users who always want to load
`relationships` data. To opt-in:

```ruby
class BlogSerializer < ActiveModel::Serializer
  associations_via_include_param(true) # opt-in to this pattern

  has_many :tags
  has_many :posts do
    link :self, '//example.com/blogs/relationships/posts'
  end
end
```

JSONAPI include parameters (http://jsonapi.org/format/#fetching-includes).

Fixes rails-api#1707
Fixes rails-api#1555
richmolj pushed a commit to richmolj/active_model_serializers that referenced this issue Jun 7, 2016
The current implementation does support conditionally sideloading
relationships based on the 'include' URL param. However, omitting the
relationship still loads the relationship (to populate the type/id
'relationships' payload), somewhat defeating the purpose. Instead, this
changes the flow to:

1. If the relationship is included, load it and include it the response.
2. If the relationship is not included but there is a JSONAPI link,
include the link in the response but do not load the relationship or
include data.
3. If the relationship is not in the URL param and there is no link, do
not include this node in the 'relationships' response.

The `current_include_tree` edits in json_api.rb are to pass the current
nested includes. This is to support when multiple entities have the same
relationship, e.g. `/blogs/?include=posts.tags,tags` should include both
blog tags and post tags, but `/blogs/?include=posts.tags` should only
include post tags.

This API is opt-in to support users who always want to load
`relationships` data. To opt-in:

```ruby
class BlogSerializer < ActiveModel::Serializer
  associations_via_include_param(true) # opt-in to this pattern

  has_many :tags
  has_many :posts do
    link :self, '//example.com/blogs/relationships/posts'
  end
end
```

JSONAPI include parameters (http://jsonapi.org/format/#fetching-includes).

Fixes rails-api#1707
Fixes rails-api#1555
richmolj pushed a commit to richmolj/active_model_serializers that referenced this issue Jun 9, 2016
The current implementation does support conditionally sideloading
relationships based on the 'include' URL param. However, omitting the
relationship still loads the relationship (to populate the type/id
'relationships' payload), somewhat defeating the purpose. Instead, this
changes the flow to:

1. If the relationship is included, load it and include it the response.
2. If the relationship is not included but there is a JSONAPI link,
include the link in the response but do not load the relationship or
include data.
3. If the relationship is not in the URL param and there is no link, do
not include this node in the 'relationships' response.

The `current_include_tree` edits in json_api.rb are to pass the current
nested includes. This is to support when multiple entities have the same
relationship, e.g. `/blogs/?include=posts.tags,tags` should include both
blog tags and post tags, but `/blogs/?include=posts.tags` should only
include post tags.

This API is opt-in to support users who always want to load
`relationships` data. To opt-in:

```ruby
class BlogSerializer < ActiveModel::Serializer
  associations_via_include_param(true) # opt-in to this pattern

  has_many :tags
  has_many :posts do
    link :self, '//example.com/blogs/relationships/posts'
  end
end
```

JSONAPI include parameters (http://jsonapi.org/format/#fetching-includes).

Fixes rails-api#1707
Fixes rails-api#1555
bf4 pushed a commit to bf4/active_model_serializers that referenced this issue Jun 10, 2016
The current implementation does support conditionally sideloading
relationships based on the 'include' URL param. However, omitting the
relationship still loads the relationship (to populate the type/id
'relationships' payload), somewhat defeating the purpose. Instead, this
changes the flow to:

1. If the relationship is included, load it and include it the response.
2. If the relationship is not included but there is a JSONAPI link,
include the link in the response but do not load the relationship or
include data.
3. If the relationship is not in the URL param and there is no link, do
not include this node in the 'relationships' response.

The `current_include_tree` edits in json_api.rb are to pass the current
nested includes. This is to support when multiple entities have the same
relationship, e.g. `/blogs/?include=posts.tags,tags` should include both
blog tags and post tags, but `/blogs/?include=posts.tags` should only
include post tags.

This API is opt-in to support users who always want to load
`relationships` data. To opt-in:

```ruby
class BlogSerializer < ActiveModel::Serializer
  associations_via_include_param(true) # opt-in to this pattern

  has_many :tags
  has_many :posts do
    link :self, '//example.com/blogs/relationships/posts'
  end
end
```

JSONAPI include parameters (http://jsonapi.org/format/#fetching-includes).

Fixes rails-api#1707
Fixes rails-api#1555
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants