Shopify / Shopify/shopify-api-ruby
Hashdiff does not see change if nested object is updated in place due to same reference value in resource object and original_state
Nobody has claimed this yet.
- Dominant language
- Ruby
- Stars
- 1.1k
- Forks
- 484
- PR merge metrics
- No merged PRs in 30d
Description
Issue summary
HashDiff does not see change if nested resource object is updated in place. When resource object is created with #create_instance method, it adds value to both resource and to the original_state. If you update the object in place, the change will be reflected in original_state aswell which is wrong.
Expected behavior
All values in original_state should be duplicates, not references.
Steps to reproduce the problem
- Install gems gem install shopify_api rspec
- Create ruby script with the content below
- Execute script with rspec bundle exec rspec ${script_file_name}
it "should reproduce issue with reference value in original_state" do
resource_hash = {
name: "#111",
line_items: [
{ title: "One", price: 100, quantity: 1 },
],
shipping_address: {
first_name: "John",
last_name: "Doe",
address_1: "Freedom street",
},
}.values_as_hash
draft_order = ShopifyAPI::DraftOrder.new(from_hash: resource_hash)
draft_order.save!
expect(draft_order.shipping_address["first_name"]).to eq("John")
# This changes original_state which is wrong
draft_order.shipping_address["first_name"] = "Tom"
draft_order.save!
expect(draft_order.shipping_address["first_name"]).to eq("Tom")
# It is only possible to update it by setting whole object like this
updated_address = draft_order.shipping_address.values_as_hash
updated_address[:first_name] = "Tom"
draft_order.shipping_address = updated_address
draft_order.save!
expect(draft_order.shipping_address["first_name"]).to eq("Tom")
end
This is our current workaround to fix this issue.
module ShopifyAPI
module Rest
module BaseExtension
def create_instance(data:, session:, instance: nil)
result = super
result.original_state = result.original_state.deep_dup
result
end
end
end
end
ShopifyAPI::Rest::Base.singleton_class.prepend(ShopifyAPI::Rest::BaseExtension)
This is where the actual issue is happening:
https://github.com/Shopify/shopify-api-ruby/blob/6a74e1e2a1ec454d700e3b53d91cae00c236fada/lib/shopify_api/rest/base.rb#L293-L296
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at lib/shopify_api/rest/base.rb lines 293-296, where create_instance builds the resource and original_state. Run the reproduction with bundle exec rspec, then verify that mutating a nested value in place does not alter original_state and is detected on save.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- ruby
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100