Shopify / shopify-api-ruby

ShopifyAPI is a lightweight gem for accessing the Shopify admin REST and GraphQL web services.
MIT License
1.05k stars 467 forks source link

Hashdiff does not see change if nested object is updated in place due to same reference value in resource object and original_state #1331

Open kaarelss opened 1 month ago

kaarelss commented 1 month ago

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

  1. Install gems gem install shopify_api rspec
  2. Create ruby script with the content below
  3. 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

Arkham commented 1 month ago

Hey 👋 ,

thanks for reporting this issue.