A case for *validating* uniqueness

Here’s an example of validating that an Event has a unique name. There may be ways to improve the code, but at least it shows what I’m talking about.

def validate_unique_name(changeset) do
  name = get_field(changeset, :name)
  if is_binary(name) do
    dups_query = from e in Event, where: e.name == ^name

    # For updates, don't flag event as a dup of itself
    id = get_field(changeset, :id)
    dups_query = if is_nil(id) do
      dups_query
    else
      from e in dups_query, where: e.id != ^id
    end

    dups = dups_query |> Reservations.Repo.all
    if Enum.any?(dups) do
      add_error(
        changeset,
        :name,
        "has already been taken",
        [validation: :validate_unique_name]
      )
    else
      # no duplicates found
      changeset
    end
  else
    # eg, name is nil
    changeset
  end
end

If this were to be implemented I don’t think you’d want to do it as part of the changeset functions themselves. Changing the purity of changesets is an enormous change. Rather you’d want to set some kind of callback in the changeset that would get called whenever you passed the changeset to Repo.insert or Repo.update

Changing the purity of changesets is an enormous change.

What, specifically, does it hurt? (I mentioned that it makes testing require a db, but that we could use separate functions for in-memory and database-dependent validations and test them separately.)

Rather you’d want to set some kind of callback in the changeset that would get called whenever you passed the changeset to Repo.insert or Repo.update

Ecto no longer supports callbacks, right? And what’s the advantage of doing it in one of the locations shown below vs the other?

    # here
    changeset = Event.changeset(%Event{}, event_params)

    # vs here
    case Repo.insert(changeset) do
      {:ok, _event} ->
      ...

You break composability.

def changeset_name(changeset, params) do
  thing
  |> cast(params, [:name])
  |> validate_unique([:name])
end

def changeset_email(changeset, params) do
  thing
  |> cast(params, [:email])
  |> validate_unique([:email])
end

if you do thing |> changeset_name(params) |> changeset_email(params) you’ve done 2 db requests when you could actually do it in just 1.

Ecto got rid of callbacks on the model / schema sure but that’s not what I’m talking about. I’m talking about using Ecto.Changeset — Ecto v3.14.0 which is already the spot for doing impure stuff with changeset.s

In general though in a functional language any time you take a process that’s pure and you make it impure it’s a pretty major breaking change.

In case of adding guard to race condition you can use database transaction to solve this problem, by using Ecto.Multi (Ecto.Multi — Ecto v3.14.0) and relaying on on_conflict call back there Ecto.Repo — Ecto v3.14.0

I hadn’t seen the on_conflict option. That would be useful if the conflicting record belonged to the same user and we wanted to just update it if it exists, but if somebody else has the username I want, the system shouldn’t update their account to have my email address and password.

if you do thing |> changeset_name(params) |> changeset_email(params) you’ve done 2 db requests when you could actually do it in just 1.

So this is bad for performance? It doesn’t seem like a huge problem to me. I suppose we could do both in one query with an OR, but then you’d have to check the resulting record(s) to see which error(s) to set.

Ecto got rid of callbacks on the model / schema sure but that’s not what I’m talking about. I’m talking about using Ecto.Changeset — Ecto v3.14.0 which is already the spot for doing impure stuff with changeset.s

The docs show using prepare_changes to update the comment_count on a post when we insert a comment. It doesn’t say whether prepare_changes runs if there are already errors on the changeset (I’m guessing not?) or whether adding errors at that point would abort the insert/update.

My goal is that if somebody’s form data has a missing email address, a password that’s too short, and a username that’s taken, we show them all of these problems at the same time. I’m happy with any approach that allows that.

I think you’re misunderstanding me.

I think your goal here is good, and worth while. I am merely suggesting that it be implemented in such a way such that the database access happens at the same point in the code we already expect it to, not somewhere else.

Whether you personally see the benefits of that or not, surely a way to get what you want with the fewest changes to others’ expectations is desirable.

I’m not saying it has to be literally prepare changes, just that it would be LIKE prepare changes in so far as its execution model is concerned.

I think you’re misunderstanding me.

Sorry.

Ah, ok. That does make sense.

I’m not saying it has to be literally prepare changes, just that it would be LIKE prepare changes in so far as its execution model is concerned.

Gotcha. You’re saying that there could be explicit ways for Ecto to enable checking for conflicting data in validations, and that would be better than my example.

But given that I’m using Ecto 2.1.3 and it doesn’t have such an affordance, maybe the best I can do is to split my validations by type explicitly for testing? Eg:

  def run_validations(struct) do
    struct
    |> validate_in_memory
    |> validate_against_database
  end

Another thing to consider is what happens with API-driven applications. In that case, I would say that the uniqueness validation being done on the server is actually not a good idea. Front-end apps normally do all sorts of validations on the form inputs, and uniqueness shouldn’t be different. How would that work? Front-end issues an additional request to check for uniqueness and show the error in the UI as soon as the field is filled. This also allows to show the error right away and not after the server round-trip of full form submission.

In that case, the changeset uniqueness validation becomes truly a fallback mechanism, and it’s completely fine that it runs only after all the other validations, since it’s going to be triggered rarely.