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

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

Open
jplaisted opened this issue Jun 6, 2023 · 9 comments
Open

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

jplaisted opened this issue Jun 6, 2023 · 9 comments

Comments

@jplaisted
Copy link

jplaisted commented Jun 6, 2023

This commit (per git bisect) 61c7f6b 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 #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
Copy link
Author

jplaisted commented Jun 6, 2023

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
Copy link
Author

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
Copy link

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
Copy link
Member

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
Copy link

@andrykonchin Is this fixed in 3.10?

@andrykonchin
Copy link
Member

No, it isn't fixed yet.

@jufemaiz
Copy link

jufemaiz commented Jul 9, 2024

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
Copy link
Member

@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
Copy link

jufemaiz commented Jul 9, 2024

@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.

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

No branches or pull requests

4 participants