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

Do not try to hydrate undeclared attributes from storage on batch_read #139

Merged

Conversation

nronas
Copy link
Contributor

@nronas nronas commented Jul 17, 2024

Issue #, if available:
N/A

Description of changes:

Hello all and thank you for the great gem. There is an asymmetry around hydrating records on read operations between canonical queries and batch reads. The asymmetry is located around undeclared attributes on the model class.

Example of model with undeclared attribute

#
# Storage has ID, name and age.
#
class Model
  include Aws::Record
  
  string_attr :ID, hash_key: true
  string_attr :name
end

On the above model when using canonical queries like Model.find(ID: '123') or Model.query(...) any undeclared attributes are omitted when building the final model instance. So in the above case we will end up with a Model instance that does not have age hydrated onto it.

But if we use a Batch Read then the build_item method will try to call #= on Nil.

NoMethodError: undefined method `=' for an instance of Model
from /Users/nronas/.rbenv/versions/3.3.3/lib/ruby/gems/3.3.0/gems/aws-record-2.13.0/lib/aws-record/record/attributes.rb:69:in `block in initialize'

This PR just skips any storage attributes that are not declared on the model class when batch reading

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@nronas nronas changed the title Do not try to hydrate undeclared attributes from the storage on batch_read Do not try to hydrate undeclared attributes from storage on batch_read Jul 17, 2024
@jterapin
Copy link
Contributor

Hi! Thank you for submitting this fix. This looks good.

Copy link
Contributor

@mullermp mullermp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution!

Gemfile Show resolved Hide resolved
@nronas
Copy link
Contributor Author

nronas commented Jul 18, 2024

@mullermp done

@mullermp
Copy link
Contributor

mullermp commented Jul 18, 2024

Sorry, this also needs a changelog entry, you can add something like

Issue - Do not try to hydrate undeclared attributes from storage on `batch_read`.
<newline>

Then it's good to go

@nronas
Copy link
Contributor Author

nronas commented Jul 18, 2024

@mullermp wasn't sure how the release process goes, will do that asap. Should this be under the Unreleased Changes section on the CHANGELOG.md?

@mullermp mullermp merged commit d4c487f into aws:main Jul 18, 2024
25 checks passed
@jterapin
Copy link
Contributor

New version of the gem is out: https://rubygems.org/gems/aws-record/versions/2.13.1

@nronas nronas deleted the on-batch-request-ignore-undefined-attributes branch July 19, 2024 07:25
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

Successfully merging this pull request may close these issues.

3 participants