Dynamoid / dynamoid

Ruby ORM for Amazon's DynamoDB.
MIT License
576 stars 197 forks source link

Field adapters no longer work (v3.9.0) #666

Open jplaisted opened 1 year ago

jplaisted commented 1 year ago

This commit (per git bisect) https://github.com/Dynamoid/dynamoid/commit/61c7f6b7e20fc21b51d8ac87efd87bac55fdc1d8 makes field adapters not longer work as expected, as changes to adapted fields are no saved in the dirty state.

Example of a failing test is in this PR https://github.com/Dynamoid/dynamoid/pull/665. This PR passes on v3.8.0 and fails on v3.9.0. git bisect determined it was the commit above, which makes sense.

tl;dr direct changes to any field :foo, FooAdapter are no longer persisted on save!

jplaisted commented 1 year ago

In some wonkiness / fairness, in 3.8.0 persisted? is in fact true before saving; it just did a put item anyway.

Edit: also, as expected, writing an entirely new object to the field works. Writing the same object does not. Added more tests to the example.

jplaisted commented 1 year ago

Haven't tested it but I'm now thinking this applies to hash fields (without any adapter) too? Anything that doesn't go through a dynamo field= (or update_attribute, etc) method.

FinnIckler commented 1 year ago

I can confirm that my custom datatype also doesn't update on Version 3.9, but does on 3.8.

registration = Registrations.find("#{competition_id}-#{user_id}")
updated_lanes = registration.lanes.map { |lane|
  if lane.name == "Competing"
    lane.lane_state = "deleted"
  end
  lane
}
puts updated_lanes.to_json # Updated here
updated_registration = registration.update_attributes!(lanes: updated_lanes) # Updated here
# But not updated in the database

There is also no PUT call in the Console Output. My model has a field :lanes, :array, of: Lane

and Lane looks like this

class Lane
  attr_accessor :name, :lane_state, :completed_steps, :step_details

  def initialize(args)
    @name = args["name"]
    @lane_state = args["lane_state"] || "waiting"
    @completed_steps = args["completed_steps"] || []
    @step_details = args["step_details"] || {}
  end

  def dynamoid_dump
    self.to_json
  end

  def self.dynamoid_load(serialized_str)
    parsed = JSON.parse serialized_str
    Lane.new(parsed)
  end
end
andrykonchin commented 1 year ago

It looks like as an overlooked broken change. Thank you for the report.

I suppose the expected behaviour is (at saving already persisted model) to persist all the attributes unconditionally if there is a field with custom type (field adapter), but not only changed/"dirty" attributes (like in works since 3.9.0)

FinnIckler commented 6 months ago

@andrykonchin Is this fixed in 3.10?

andrykonchin commented 5 months ago

No, it isn't fixed yet.

jufemaiz commented 2 months ago

Haven't tested it but I'm now thinking this applies to hash fields (without any adapter) too? Anything that doesn't go through a dynamo field= (or update_attribute, etc) method.

I'm also seeing failure to update using update_attribute directly and by using field= approach. Downgrading to v3.8.0 is the only solution.

andrykonchin commented 2 months ago

@jufemaiz Could you please provide examples of code that don't work as expected to save an attribute new value with update_attribute and <field>= setter to reproduce the issue?

jufemaiz commented 2 months ago

@jufemaiz Could you please provide examples of code that don't work as expected to save an attribute new value with update_attribute and <field>= setter to reproduce the issue?

I'll work on an example that's not dependent on internal repos.