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.

Sign up for GitHub

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

Jump to bottom

@only_fields and @except_fields memoization is wrong #149

Closed
marshall-lee opened this issue Jul 21, 2015 · 1 comment
Closed

@only_fields and @except_fields memoization is wrong #149

marshall-lee opened this issue Jul 21, 2015 · 1 comment
Labels
bug?

Comments

@marshall-lee
Copy link
Member

only_fields and except_fields methods internally memoizing @only_fields and @except_fields instance variables respectively. It's wrong because they're calculated against passed options parameter which can differ between separate serializable_hash calls.

I'd open a PR later.
@dblock dblock added the bug? label Jul 21, 2015
marshall-lee added a commit to marshall-lee/grape-entity that referenced this issue Jul 21, 2015
@marshall-lee
Refactoring: extract exposures.
d864b10 
This one reaches many goals at once:

 - Fixes ruby-grape#56: now `exposures` is not a hash table but an array.
 - Due to previous point, tree structure `nested_exposures` based on
   flat hash table with keys like `root_node__node__node` is removed
   because now double exposures don't rewrite previously defined
   so such tree structure is simply incorrect.
   `NestingExposure` with an array of children is introduced instead.
 - For ones who want previous rewriting behavior a `rewrite` option
   for `exposure` is introduced.
 - Fixes ruby-grape#149: runtime `options` are now wrapped in an `Options` object.
 - Fixes ruby-grape#150: see new `spec/grape_entity/exposure_spec.rb`
 - All exposure configuration and exposing strategies are extracted to
   the separate classes.
 - `key_for`, `name_for` internal methods are removed.
 - Much of the overhead is gone so performance is increased by 20-30%.
@marshall-lee marshall-lee mentioned this issue Jul 21, 2015
Merged
marshall-lee added a commit to marshall-lee/grape-entity that referenced this issue Jul 21, 2015
@marshall-lee
Refactoring: extract exposures.
edb8467 
This one reaches many goals at once:

 - Fixes ruby-grape#56: now `exposures` is not a hash table but an array.
 - Due to previous point, tree structure `nested_exposures` based on
   flat hash table with keys like `root_node__node__node` is removed
   because now double exposures don't rewrite previously defined
   so such tree structure is simply incorrect.
   `NestingExposure` with an array of children is introduced instead.
 - For ones who want previous rewriting behavior a `rewrite` option
   for `exposure` is introduced.
 - Fixes ruby-grape#149: runtime `options` are now wrapped in an `Options` object.
 - Fixes ruby-grape#150: see new `spec/grape_entity/exposure_spec.rb`
 - All exposure configuration and exposing strategies are extracted to
   the separate classes.
 - `key_for`, `name_for` internal methods are removed.
 - Much of the overhead is gone so performance is increased by 20-30%.
marshall-lee added a commit to marshall-lee/grape-entity that referenced this issue Jul 22, 2015
@marshall-lee
Refactoring: extract exposures.
f7bc1e5 
This one reaches many goals at once:

 - Fixes ruby-grape#56: now `exposures` is not a hash table but an array.
 - Due to previous point, tree structure `nested_exposures` based on
   flat hash table with keys like `root_node__node__node` is removed
   because now double exposures don't rewrite previously defined
   so such tree structure is simply incorrect.
   `NestingExposure` with an array of children is introduced instead.
 - For ones who want previous rewriting behavior a `rewrite` option
   for `exposure` is introduced.
 - Fixes ruby-grape#149: runtime `options` are now wrapped in an `Options` object.
 - Fixes ruby-grape#150: see new `spec/grape_entity/exposure_spec.rb`
 - All exposure configuration and exposing strategies are extracted to
   the separate classes.
 - `key_for`, `name_for` internal methods are removed.
 - Much of the overhead is gone so performance is increased by 20-30%.
marshall-lee added a commit to marshall-lee/grape-entity that referenced this issue Jul 23, 2015
@marshall-lee
Refactoring: extract exposures.
1dba59e 
This one reaches many goals at once:

 - Fixes ruby-grape#56: now `exposures` is not a hash table but an array.
 - Due to previous point, tree structure `nested_exposures` based on
   flat hash table with keys like `root_node__node__node` is removed
   because now double exposures don't rewrite previously defined
   so such tree structure is simply incorrect.
   `NestingExposure` with an array of children is introduced instead.
 - Ones who want an old rewriting behavior should manually `unexpose`.
 - Fixes ruby-grape#112
 - Fixes ruby-grape#149: runtime `options` are now wrapped in an `Options` object.
 - Fixes ruby-grape#150: see new `spec/grape_entity/exposure_spec.rb`
 - Fixes ruby-grape#152
 - Fixes ruby-grape#153
 - Fixes ruby-grape#155
 - All exposure configuration and exposing strategies are extracted to
   the separate classes.
 - `key_for`, `name_for` internal methods are removed.
 - Much of the overhead is gone so performance is increased by 20-30%.
marshall-lee added a commit to marshall-lee/grape-entity that referenced this issue Jul 23, 2015
@marshall-lee
Refactoring: extract exposures.
76803a8 
This one reaches many goals at once:

 - Fixes ruby-grape#56: now `exposures` is not a hash table but an array.
 - Due to previous point, tree structure `nested_exposures` based on
   flat hash table with keys like `root_node__node__node` is removed
   because now double exposures don't rewrite previously defined
   so such tree structure is simply incorrect.
   `NestingExposure` with an array of children is introduced instead.
 - Ones who want an old rewriting behavior should manually `unexpose`.
 - Fixes ruby-grape#112
 - Fixes ruby-grape#149: runtime `options` are now wrapped in an `Options` object.
 - Fixes ruby-grape#150: see new `spec/grape_entity/exposure_spec.rb`
 - Fixes ruby-grape#152
 - Fixes ruby-grape#153
 - Fixes ruby-grape#155
 - All exposure configuration and exposing strategies are extracted to
   the separate classes.
 - `key_for`, `name_for` internal methods are removed.
 - Much of the overhead is gone so performance is increased by 20-30%.
@marshall-lee
Copy link
Member Author

Failing example:

context 'with projections passed in options' do
  it 'allows to pass different :only and :except params using the same instance' do
    fresh_class.expose :a, :b, :c
    presenter = fresh_class.new({ a: 1, b: 2, c: 3})
    expect(presenter.serializable_hash(only: [:a, :b])).to eq(a: 1, b: 2)
    expect(presenter.serializable_hash(only: [:b, :c])).to eq(b: 2, c: 3)
  end
end

marshall-lee added a commit to marshall-lee/grape-entity that referenced this issue Jul 24, 2015
@marshall-lee
Refactoring: extract exposures.
aaad470 
This one reaches many goals at once:

 - Fixes ruby-grape#56.
 - `exposures` hash table is removed and substituted with
   `root_exposures` array.
 - Due to previous point, tree structure `nested_exposures` based on
   flat hash table with keys like `root_node__node__node` is removed
   because now double exposures don't rewrite previously defined
   so such tree structure is simply incorrect.
   `NestingExposure` with an array of children is introduced instead.
 - Ones who want an old rewriting behavior should manually `unexpose`.
 - Fixes ruby-grape#112.
 - Fixes ruby-grape#149: runtime `options` are now wrapped in an `Options` object.
 - Fixes ruby-grape#150: see new `spec/grape_entity/exposure_spec.rb`.
 - Fixes ruby-grape#152.
 - Fixes ruby-grape#153.
 - Fixes ruby-grape#155.
 - Fixes ruby-grape#156.
 - All exposure configuration and exposing strategies are extracted to
   the separate classes.
 - `key_for`, `name_for` internal methods are removed.
 - Much of the overhead is gone so performance is increased by 20-30%.

Version is bumbed to 0.5.0.
marshall-lee added a commit to marshall-lee/grape-entity that referenced this issue Jul 24, 2015
@marshall-lee
Refactoring: extract exposures.
be25713 
This one reaches many goals at once:

 - Fixes ruby-grape#56.
 - `exposures` hash table is removed and substituted with
   `root_exposures` array.
 - Due to previous point, tree structure `nested_exposures` based on
   flat hash table with keys like `root_node__node__node` is removed
   because now double exposures don't rewrite previously defined
   so such tree structure is simply incorrect.
   `NestingExposure` with an array of children is introduced instead.
 - Ones who want an old rewriting behavior should manually `unexpose`.
 - Fixes ruby-grape#112.
 - Fixes ruby-grape#149: runtime `options` are now wrapped in an `Options` object.
 - Fixes ruby-grape#150: see new `spec/grape_entity/exposure_spec.rb`.
 - Fixes ruby-grape#152.
 - Fixes ruby-grape#153.
 - Fixes ruby-grape#155.
 - Fixes ruby-grape#156.
 - All exposure configuration and exposing strategies are extracted to
   the separate classes.
 - `key_for`, `name_for` internal methods are removed.
 - Much of the overhead is gone so performance is increased by 20-30%.

Version is bumbed to 0.5.0.
marshall-lee added a commit to marshall-lee/grape-entity that referenced this issue Jul 24, 2015
@marshall-lee
Refactoring: extract exposures.
3df0720 
This one reaches many goals at once:

 - Fixes ruby-grape#56.
 - `exposures` hash table is removed and substituted with
   `root_exposures` array.
 - Due to previous point, tree structure `nested_exposures` based on
   flat hash table with keys like `root_node__node__node` is removed
   because now double exposures don't rewrite previously defined
   so such tree structure is simply incorrect.
   `NestingExposure` with an array of children is introduced instead.
 - Ones who want an old rewriting behavior should manually `unexpose`.
 - Fixes ruby-grape#112.
 - Fixes ruby-grape#149: runtime `options` are now wrapped in an `Options` object.
 - Fixes ruby-grape#150: see new `spec/grape_entity/exposure_spec.rb`.
 - Fixes ruby-grape#152.
 - Fixes ruby-grape#153.
 - Fixes ruby-grape#155.
 - Fixes ruby-grape#156.
 - All exposure configuration and exposing strategies are extracted to
   the separate classes.
 - `key_for`, `name_for` internal methods are removed.
 - Much of the overhead is gone so performance is increased by 20-30%.

Version is bumbed to 0.5.0.
marshall-lee added a commit to marshall-lee/grape-entity that referenced this issue Jul 27, 2015
@marshall-lee
Refactoring: extract exposures.
b819f13 
This one reaches many goals at once:

 - Fixes ruby-grape#56.
 - `exposures` hash table is removed and substituted with
   `root_exposures` array.
 - Due to previous point, tree structure `nested_exposures` based on
   flat hash table with keys like `root_node__node__node` is removed
   because now double exposures don't rewrite previously defined
   so such tree structure is simply incorrect.
   `NestingExposure` with an array of children is introduced instead.
 - Ones who want an old rewriting behavior should manually `unexpose`.
 - Fixes ruby-grape#112.
 - Fixes ruby-grape#149: runtime `options` are now wrapped in an `Options` object.
 - Fixes ruby-grape#150: see new `spec/grape_entity/exposure_spec.rb`.
 - Fixes ruby-grape#152.
 - Fixes ruby-grape#153.
 - Fixes ruby-grape#155.
 - Fixes ruby-grape#156.
 - All exposure configuration and exposing strategies are extracted to
   the separate classes.
 - `key_for`, `name_for` internal methods are removed.
 - Much of the overhead is gone so performance is increased by 20-30%.
marshall-lee added a commit to marshall-lee/grape-entity that referenced this issue Jul 27, 2015
@marshall-lee
Refactoring: extract exposures.
6c395d0 
This one reaches many goals at once:

 - Fixes ruby-grape#56.
 - `exposures` hash table is removed and substituted with
   `root_exposures` array.
 - Due to previous point, tree structure `nested_exposures` based on
   flat hash table with keys like `root_node__node__node` is removed
   because now double exposures don't rewrite previously defined
   so such tree structure is simply incorrect.
   `NestingExposure` with an array of children is introduced instead.
 - Ones who want an old rewriting behavior should manually `unexpose`.
 - Fixes ruby-grape#112.
 - Fixes ruby-grape#149: runtime `options` are now wrapped in an `Options` object.
 - Fixes ruby-grape#150: see new `spec/grape_entity/exposure_spec.rb`.
 - Fixes ruby-grape#152.
 - Fixes ruby-grape#153.
 - Fixes ruby-grape#155.
 - Fixes ruby-grape#156.
 - All exposure configuration and exposing strategies are extracted to
   the separate classes.
 - `key_for`, `name_for` internal methods are removed.
 - Much of the overhead is gone so performance is increased by 20-30%.
marshall-lee added a commit to marshall-lee/grape-entity that referenced this issue Jul 27, 2015
@marshall-lee
Refactoring: extract exposures.
0983a92 
This one reaches many goals at once:

 - Fixes ruby-grape#56.
 - `exposures` hash table is removed and substituted with
   `root_exposures` array.
 - Due to previous point, tree structure `nested_exposures` based on
   flat hash table with keys like `root_node__node__node` is removed
   because now double exposures don't rewrite previously defined
   so such tree structure is simply incorrect.
   `NestingExposure` with an array of children is introduced instead.
 - Ones who want an old rewriting behavior should manually `unexpose`.
 - Fixes ruby-grape#112.
 - Fixes ruby-grape#149: runtime `options` are now wrapped in an `Options` object.
 - Fixes ruby-grape#150: see new `spec/grape_entity/exposure_spec.rb`.
 - Fixes ruby-grape#152.
 - Fixes ruby-grape#153.
 - Fixes ruby-grape#155.
 - Fixes ruby-grape#156.
 - All exposure configuration and exposing strategies are extracted to
   the separate classes.
 - `key_for`, `name_for` internal methods are removed.
 - Much of the overhead is gone so performance is increased by 20-30%.
marshall-lee added a commit to marshall-lee/grape-entity that referenced this issue Jul 27, 2015
@marshall-lee
Refactoring: extract exposures.
050e965 
This one reaches many goals at once:

 - Fixes ruby-grape#56.
 - `exposures` hash table is removed and substituted with
   `root_exposures` array.
 - Due to previous point, tree structure `nested_exposures` based on
   flat hash table with keys like `root_node__node__node` is removed
   because now double exposures don't rewrite previously defined
   so such tree structure is simply incorrect.
   `NestingExposure` with an array of children is introduced instead.
 - Ones who want an old rewriting behavior should manually `unexpose`.
 - Fixes ruby-grape#112.
 - Fixes ruby-grape#149: runtime `options` are now wrapped in an `Options` object.
 - Fixes ruby-grape#150: see new `spec/grape_entity/exposure_spec.rb`.
 - Fixes ruby-grape#152.
 - Fixes ruby-grape#153.
 - Fixes ruby-grape#155.
 - Fixes ruby-grape#156.
 - All exposure configuration and exposing strategies are extracted to
   the separate classes.
 - `key_for`, `name_for` internal methods are removed.
 - Much of the overhead is gone so performance is increased by 20-30%.
marshall-lee added a commit to marshall-lee/grape-entity that referenced this issue Aug 2, 2015
@marshall-lee
Refactoring: extract exposures.
c550978 
This one reaches many goals at once:

 - Fixes ruby-grape#56.
 - `exposures` hash table is removed and substituted with
   `root_exposures` array.
 - Due to previous point, tree structure `nested_exposures` based on
   flat hash table with keys like `root_node__node__node` is removed
   because now double exposures don't rewrite previously defined
   so such tree structure is simply incorrect.
   `NestingExposure` with an array of children is introduced instead.
 - Ones who want an old rewriting behavior should manually `unexpose`.
 - Fixes ruby-grape#112.
 - Fixes ruby-grape#149: runtime `options` are now wrapped in an `Options` object.
 - Fixes ruby-grape#150: see new `spec/grape_entity/exposure_spec.rb`.
 - Fixes ruby-grape#152.
 - Fixes ruby-grape#153.
 - Fixes ruby-grape#155.
 - Fixes ruby-grape#156.
 - All exposure configuration and exposing strategies are extracted to
   the separate classes.
 - `key_for`, `name_for` internal methods are removed.
 - Much of the overhead is gone so performance is increased by 15-30%.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug?
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dblock @marshall-lee