Check if an identity exists in a before_action

rohan
2023-04-08

rohan:

I’m not quite sure how to accomplish this in Ash:

I’m uploading files to GCS and want to make sure if the hash of the file matches one I already have, I don’t upload it again. The upload happens in a before_action on create right now. The hash of the file is an identity, and I thought eager checking would prevent the upload but it doesn’t seem to.

I’d like the create_with_binary to return the original record if it exists (without uploading) or upload and create a new record if necessary. So basically find_or_create .

Here’s the code so far:

defmodule Scribble.EmailHandler.AudioFile do
  use Ash.Resource, data_layer: AshPostgres.DataLayer

  postgres do
    table "audio_files"
    repo(Scribble.Repo)
  end

  attributes do
    uuid_primary_key :id
    attribute :hash, :string, allow_nil?: false
    attribute :url, :string, allow_nil?: false
  end

  identities do
    identity :hash, [:hash], eager_check_with: Scribble.EmailHandler
  end

  code_interface do
    define_for Scribble.EmailHandler

    define :create_with_binary, args: [:audio_file]
  end

  actions do
    defaults [:read]

    create :create_with_binary do
      argument :audio_file, :map, allow_nil?: false
      accept []

      change fn changeset, _context ->
        Ash.Changeset.before_action(changeset, fn changeset ->
          audio_file = Ash.Changeset.get_argument(changeset, :audio_file)
          filename = audio_file_hash(audio_file)

          {:ok, ^filename} =
            Scribble.Recording.store(%{filename: filename, binary: audio_file.binary})

          url = Scribble.Recording.url(filename)
          Ash.Changeset.change_attributes(changeset, %{url: url, hash: filename})
        end)
      end
    end
  end

  defp audio_file_hash(%{binary: binary, extension: extension}) do
    hash = Scribble.Scribe.audio_hash(binary)
    "#{hash}#{extension}"
  end
end

ZachDaniel:

create :create_with_binary do
  upsert? true
  upsert_identity :hash

  ...rest of your action
end

ZachDaniel:

I think that might be all you need

rohan:

i wasn’t able to find many docs on the upsert - does that prevent the before_action from running?

ZachDaniel:

eager_check_with will do its work before before_action hooks are run, which is why you probably saw no change on that

ZachDaniel:

Nope

ZachDaniel:

Ah, I see

rohan:

i think i might need to manually do a read of the database and then block the rest of the action from executing, returning the thing that was read

rohan:

but not sure how to do that in the before_action

ZachDaniel:

you need to make sure not to upsert if one exists already.

 create :create_with_binary do
      argument :audio_file, :map, allow_nil?: false
      accept []

      change fn changeset, _context ->
        Ash.Changeset.before_action(changeset, fn changeset ->
          audio_file = Ash.Changeset.get_argument(changeset, :audio_file)
          filename = audio_file_hash(audio_file)
          case <get_by_hash> do
             {:ok, result} ->
               Ash.Changeset.set_result(changeset, result)
             {:error, _} ->
               do_the_upload
          end
        end)
      end
    end

ZachDaniel:

Ash.Changeset.set_result lets you have a sort of “conditional” manual action, where sometimes the result is computed by you and other times you let Ash execute the action.

rohan:

ooh that’s really cool haha

rohan:

trying it now

rohan:

silly thing but is there a way to call get on the AudioFile and not the associated API

rohan:

like rather than: Scribble.EmailHandler.get(AudioFile, hash: filename)

ZachDaniel:

Have you looked into the code interface yet?

ZachDaniel:

code_interface do
  define_for Your.Api

  define :get_by_hash, action: :read, get_by: [:hash], args: [:hash]
end

will get you

Scribble.EmailHandler.get_by_hash(hash)

rohan:

oh yes I’ve done that but I meant rather than having to use Scribe.EmailHandler.get_by_hash or Scribe.EmailHandler.get I’d want to do : Scribe.EmailHandler.AudioFile.get or something

rohan:

it’s a small thing but it feels like an AudioFile should know how to get itself

ZachDaniel:

Oh, interesting

rohan:

in case I move it somewhere else later

ZachDaniel:

Nothing to do that currently, although you can always do

def get(fields, opts) do
  YourApi.get(fields, opts)
end

ZachDaniel:

in your resource

ZachDaniel:

One thing you may need to do is lock the record in the database to prevent multiple of these actions happening side by side. Ash doesn’t currently support that natively, so you may want to do something like this:

read :get_and_lock_by_hash do
  get? true

  argument :hash, :string, allow_nil?: false
  filter expr(hash == ^arg(:hash))
  modify_query &lock_for_update/2
end

...

defp lock_for_update(_, query) do
  {:ok, Ecto.Query.lock(query, "FOR UPDATE")}
end

code_interface do
  read :get_and_lock_by_hash, args: [:hash]
end

rohan:

it’s a little confusing to me that :


  code_interface do
    define_for Scribble.EmailHandler

    define :create_with_binary, args: [:audio_file]
    define :get_by_hash, action: :read, get_by: [:hash]
  end

this defines functions on Scribble.EmailHandler.

when it says define_for Scribble.EmailHandler

rohan:

i like that behavior I just don’t quite get the syntax

ZachDaniel:

Yes, define_for is which api the generated functions will call under the hood

ZachDaniel:

because all action invocations always go through an api module

rohan:

ah so that way any extensions in the API apply to that resource

ZachDaniel:

Yep, and there are certain rules like authorization configuration/timeouts and things like that that can be configured at the api level

rohan:

i’d love a video on how ash works under the hood whenever you get to it 🙂 always helps me understand how to use something properly when I get the mental model behind it

ZachDaniel:

For sure 🙂

ZachDaniel:

We’re also working to make that code generally more understandable by using Ash.Flow

ZachDaniel:

Ash.Flow can auto generate flow charts for a given operation, and so ideally we can create an Ash.Flow for the various parts of our system, and then explain any given thing by generating the diagram of it

ZachDaniel:

or at least allow for a way to see that behavior laid out without having to read the code

ZachDaniel:

but that will probably be a while 😄

rohan:

this didn’t seem to work

rohan:

I’m getting the error that :url is required, :hash is required

ZachDaniel:

oh interesting…

ZachDaniel:

See if this helps

ZachDaniel:

actually hang on…

ZachDaniel:

is there a stacktrace there?

ZachDaniel:

There is probably a check we are doing that we shouldn’t do if set_result has been used 🙂

rohan:

      change fn changeset, _context ->
        Ash.Changeset.before_action(changeset, fn changeset ->
          audio_file = Ash.Changeset.get_argument(changeset, :audio_file)
          filename = audio_file_hash(audio_file)

          case Scribble.EmailHandler.AudioFile.get_by_hash(filename) do
            {:ok, audio_file} ->
              IO.puts("EXISTS")
              IO.inspect(audio_file)
              Ash.Changeset.set_result(changeset, audio_file) |> IO.inspect()

            {:error, _} ->
              {:ok, ^filename} =
                Scribble.Recording.store(%{filename: filename, binary: audio_file.binary})

              url = Scribble.Recording.url(filename)
              Ash.Changeset.change_attributes(changeset, %{url: url, hash: filename})
          end
        end)
      end

rohan:

that’s my code right now

rohan:

no stacktrace

rohan:

it’s returning an {:error, %Ash.Error.Invalid...}

ZachDaniel:

can you use the ! version of the function you’re calling?

ZachDaniel:

should see some stacktraces there

rohan:

↳ anonymous fn/3 in Ash.Changeset.with_hooks/3, at: lib/ash/changeset/changeset.ex:1573
** (Ash.Error.Invalid) Input Invalid

* attribute url is required
  (ash 2.6.29) lib/ash/changeset/changeset.ex:1425: anonymous fn/2 in Ash.Changeset.require_values/4
  (elixir 1.14.3) lib/enum.ex:2468: Enum."-reduce/3-lists^foldl/2-0-"/3
  (ash 2.6.29) lib/ash/actions/create.ex:383: anonymous fn/8 in Ash.Actions.Create.as_requests/5
  (ash 2.6.29) lib/ash/changeset/changeset.ex:1865: Ash.Changeset.run_around_actions/2
  (ash 2.6.29) lib/ash/changeset/changeset.ex:1575: anonymous fn/2 in Ash.Changeset.with_hooks/3
  (ecto_sql 3.9.2) lib/ecto/adapters/sql.ex:1203: anonymous fn/3 in Ecto.Adapters.SQL.checkout_or_transaction/4
  (db_connection 2.4.3) lib/db_connection.ex:1611: DBConnection.run_transaction/4
  (ash 2.6.29) lib/ash/changeset/changeset.ex:1573: anonymous fn/3 in Ash.Changeset.with_hooks/3
  (ash 2.6.29) lib/ash/changeset/changeset.ex:1689: Ash.Changeset.transaction_hooks/2
  (ash 2.6.29) lib/ash/actions/create.ex:319: anonymous fn/10 in Ash.Actions.Create.as_requests/5
  (ash 2.6.29) lib/ash/engine/request.ex:1048: Ash.Engine.Request.do_try_resolve_local/4
  (ash 2.6.29) lib/ash/engine/request.ex:282: Ash.Engine.Request.do_next/1
  (ash 2.6.29) lib/ash/engine/request.ex:211: Ash.Engine.Request.next/1
  (ash 2.6.29) lib/ash/engine/engine.ex:683: Ash.Engine.advance_request/2
  (ash 2.6.29) lib/ash/engine/engine.ex:589: Ash.Engine.fully_advance_request/2
  (ash 2.6.29) lib/ash/engine/engine.ex:530: Ash.Engine.do_run_iteration/2
  (elixir 1.14.3) lib/enum.ex:2468: Enum."-reduce/3-lists^foldl/2-0-"/3
  (ash 2.6.29) lib/ash/engine/engine.ex:271: Ash.Engine.run_to_completion/1
* attribute hash is required
  (ash 2.6.29) lib/ash/changeset/changeset.ex:1425: anonymous fn/2 in Ash.Changeset.require_values/4
  (elixir 1.14.3) lib/enum.ex:2468: Enum."-reduce/3-lists^foldl/2-0-"/3
  (ash 2.6.29) lib/ash/actions/create.ex:383: anonymous fn/8 in Ash.Actions.Create.as_requests/5
  (ash 2.6.29) lib/ash/changeset/changeset.ex:1865: Ash.Changeset.run_around_actions/2
  (ash 2.6.29) lib/ash/changeset/changeset.ex:1575: anonymous fn/2 in Ash.Changeset.with_hooks/3
  (ecto_sql 3.9.2) lib/ecto/adapters/sql.ex:1203: anonymous fn/3 in Ecto.Adapters.SQL.checkout_or_transaction/4
  (db_connection 2.4.3) lib/db_connection.ex:1611: DBConnection.run_transaction/4
  (ash 2.6.29) lib/ash/changeset/changeset.ex:1573: anonymous fn/3 in Ash.Changeset.with_hooks/3
  (ash 2.6.29) lib/ash/changeset/changeset.ex:1689: Ash.Changeset.transaction_hooks/2
  (ash 2.6.29) lib/ash/actions/create.ex:319: anonymous fn/10 in Ash.Actions.Create.as_requests/5
  (ash 2.6.29) lib/ash/engine/request.ex:1048: Ash.Engine.Request.do_try_resolve_local/4
  (ash 2.6.29) lib/ash/engine/request.ex:282: Ash.Engine.Request.do_next/1
  (ash 2.6.29) lib/ash/engine/request.ex:211: Ash.Engine.Request.next/1
  (ash 2.6.29) lib/ash/engine/engine.ex:683: Ash.Engine.advance_request/2
  (ash 2.6.29) lib/ash/engine/engine.ex:589: Ash.Engine.fully_advance_request/2
  (ash 2.6.29) lib/ash/engine/engine.ex:530: Ash.Engine.do_run_iteration/2
  (elixir 1.14.3) lib/enum.ex:2468: Enum."-reduce/3-lists^foldl/2-0-"/3
  (ash 2.6.29) lib/ash/engine/engine.ex:271: Ash.Engine.run_to_completion/1
    (ash 2.6.29) lib/ash/error/error.ex:463: Ash.Error.choose_error/2
    (ash 2.6.29) lib/ash/error/error.ex:218: Ash.Error.to_error_class/2
    (ash 2.6.29) lib/ash/actions/create.ex:126: Ash.Actions.Create.do_run/4
    (ash 2.6.29) lib/ash/actions/create.ex:38: Ash.Actions.Create.run/4
    (ash 2.6.29) lib/ash/api/api.ex:1476: Ash.Api.create!/3
    iex:36: (file)

rohan:

what’s interesting is I’m inspecting the changeset after set_result and the data does look like it’s nil for some reason

rohan:

#Ash.Changeset<
  api: Scribble.EmailHandler,
  action_type: :create,
  action: :create_with_binary,
  attributes: %{id: "6523c5c6-d18c-4867-8a21-d4ba7805d97a"},
  relationships: %{},
  arguments: %{audio_file: %{binary: <<255>>, extension: ".mp3"}},
  errors: [],
  data: #Scribble.EmailHandler.AudioFile<
    __meta__: #Ecto.Schema.Metadata<:built, "audio_files">,
    id: nil,
    hash: nil,
    url: nil,
    aggregates: %{},
    calculations: %{},
    __order__: nil,
    ...
  >,
  context: %{actor: nil, authorize?: false},
  valid?: true
>

ZachDaniel:

Yeah, thats fine 🙂

ZachDaniel:

The way we set the result is into the context key which is hidden there

ZachDaniel:

well, trimmed down

ZachDaniel:

can you try main ?

ZachDaniel:

I just pushed something up that ought to fix it

rohan:

yup trying it now

rohan:

hmm did something change between main and 2.6 about how code_interfaces work

rohan:

my case clause is failing

ZachDaniel:

🤔 don’t think so

rohan:

does it no longer return {:ok, …}

ZachDaniel:

If so its a bug

ZachDaniel:

did you add a ! to the call?

ZachDaniel:

that will cause it not to return {:ok,

rohan:

nope

ZachDaniel:

🤔 so get_by_hash is in your code interface, and its returning %Record{} ?

rohan:

yes

rohan:

but on failure it’s returning an {:error, _}

rohan:

here’s the case:

          case Scribble.EmailHandler.AudioFile.get_by_hash(filename) do
            {:error, _} ->
              {:ok, ^filename} =
                Scribble.Recording.store(%{filename: filename, binary: audio_file.binary})

              url = Scribble.Recording.url(filename)
              Ash.Changeset.change_attributes(changeset, %{url: url, hash: filename})

            audio_file ->
              Ash.Changeset.set_result(changeset, audio_file)
          end

rohan:

^ this works

ZachDaniel:

🤔

rohan:

and here’s the code interface:

  code_interface do
    define_for Scribble.EmailHandler

    define :create_with_binary, args: [:audio_file]
    define :get_by_hash, action: :read, get_by: [:hash]
  end

ZachDaniel:

oh, actually are you sure its your case statement?

ZachDaniel:

I think its because you need to do Ash.Changeset.set_result(changeset, {:ok, audio_file})

ZachDaniel:

I forgot about that

rohan:

that worked!

ZachDaniel:

Yeah, so it was actually the case statement in the create function

rohan:

cool everything’s working now

rohan:

🙂

rohan:

thanks!

ZachDaniel:

👍