Atomic updates across embedded and normal attributes, failing since 3.5.10

We have an error that appeared in 3.5.9 → 3.5.10. I think this code specifically: fix: properly prevent embedded attribute updates in atomics · ash-project/ash@e635e57 · GitHub

We have an update action that covers a resource attribute (User.email) and an embedded set of attributes (User.settings) at the same time, updating via a form.

    update :update_settings do
      accept [:settings, :email]
    end
defmodule Wink.Accounts.User.Settings do
  @moduledoc false

  use Ash.Resource,
    domain: Wink.Accounts,
    data_layer: :embedded

  attributes do
    attribute :first_name, :string do
      public? true
    end

    attribute :last_name, :string do
      public? true
    end

    attribute :date_format, :string do
      public? true
      default "%d/%m/%Y"
      # one_of constraint would be clearer, but is supported for atom values only
      # changing this attribute to an atom would require an extra logic to covert back to string
      # this "not-so-easy-to-read" regex currently seems the lesser evil
      # e.g. "%d/%m/%Y" or "%m/%d/%Y"
      constraints match: ~r/^%d\/%m\/%Y|%m\/%d\/%Y$/
    end
  end

  calculations do
    calculate :full_name, :string, expr(first_name <> " " <> last_name)
  end
end

The embedded resource doesn’t have a primary key, but this is now failing. Is this an edge case, or theoretically should this update across the two attribute types be supported?

It can’t work atomically because we don’t have logic to update (merge) the underlying maps atomically. If you are always supplying the full new settings every time you update it you can set the constraint on_update: :replace to signal that. It was not correct for us to be letting you do it before :sweat_smile:

1 Like

@zachdaniel we’ve tried to apply this constraint but can’t seem to resolve the Embedded attributes do not support atomic updates error. We’ve recreated a minimal app here - GitHub - alexslade/atomicity_bug - the readme should explain it all.

I don’t want to raise this as a bug until someone at least confirms it is, but if that’s so we can put some time into resolving it. I suspect we might just have config in the wrong place?

Add require_atomic?(false) to your update_settings action. Since it can’t be atomic it won’t be atomic. :cowboy_hat_face:

I understand that’s a workaround that would mark the entire action as non-atomic. I think there’s a bug here which I’d like to understand, and it would be nice to maintain the atomic action even if the “settings” attribute isn’t treated atomically.

This is on my list to look at soon. I’m not at a computer, but I believe the bug is on this line (debugging is hard on phone): ash/lib/ash/embeddable_type.ex at v3.5.12 · ash-project/ash · GitHub

I believe that should be !list? || (list? and ...) on that line. Maybe that can unblock you for now if you get a chance to try that out :slight_smile:

This does indeed fix it, thanks for the pointer. On monday I’ll see if I can add a test to cover this and raise an issue PR.

2 Likes