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

@documentation memoization is slightly incorrect #153

Closed
marshall-lee opened this issue Jul 23, 2015 · 0 comments
Closed

@documentation memoization is slightly incorrect #153

marshall-lee opened this issue Jul 23, 2015 · 0 comments
Labels

Comments

@marshall-lee
Copy link
Member

def self.documentation
  @documentation ||= exposures.each_with_object({}) do |(attribute, exposure_options), memo|
    if exposure_options[:documentation].present?
      memo[key_for(attribute)] = exposure_options[:documentation]
    end
  end
end

It works incorrect in this corner case:

entity_class = Class.new(Grape::Entity)
entity_class.expose :x, documentation: { desc: 'just x' }
doc1 = entity_class.documentation
entity_class.expose :y, documentation: { desc: 'just y' }
doc2 = entity_class.documentation
doc1 # => {:x=>{:desc=>"just x"}} 
doc1 == doc2 # => true

It could be fixed simply by assigning nil to @documentation in expose and unexpose.
Will be fixed in #151

@dblock dblock added the bug? label Jul 23, 2015
marshall-lee added a commit to marshall-lee/grape-entity that referenced this issue Jul 23, 2015
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
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 24, 2015
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
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
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
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
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
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
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
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
Labels
Projects
None yet
Development

No branches or pull requests

2 participants