Search code examples
validationelixirectochangeset

With Ecto, validate that a changeset with 2 different related models have the same parent model


In my app I have a method to create a new response. A response has a belongs_to relationship to both a player and match.

In addition player and match both have a belongs_to relationship to a team.

It looks like this:

schema

When inserting a new response I want to validate that the player and match having the player_id and match_id foreign keys in the changeset belong to the same team.

Currently I'm achieving this as follows. First, define a custom validation that checks the records belonging to the foreign keys:

def validate_match_player(changeset) do
  player_team =
    Player
    |> Repo.get(get_field(changeset, :player_id))
    |> Map.get(:team_id)

  match_team =
    Match
    |> Repo.get(get_field(changeset, :match_id))
    |> Map.get(:team_id)

  cond do
    match_team == player_team -> changeset
    true -> changeset |> add_error(:player, "does not belong to the same team as the match")
  end
end

and the use the validation as part of the changeset:

def changeset(model, params \\ %{}) do
  model
  |> cast(params, [:player_id, :match_id, :message])
  |> validate_required([:player_id, :match_id, :message])
  |> foreign_key_constraint(:match_id)
  |> foreign_key_constraint(:player_id)
  |> validate_match_player()
  |> unique_constraint(
    :player,
    name: :responses_player_id_match_id_unique,
    message: "already has an response for this match"
  )
end

This works fine but involves a couple of extra SQL queries to look up the related records in order to get their team_id foreign keys to compare them.

Is there a nicer way to do this, perhaps using constraints, that avoids the extra queries?


Solution

  • I have two possible improvements:

    • Application level solution: instead of two queries, you just query once.
    • Database level solution: you create a trigger for the check in the database.

    Application Level Solution

    Right now you have two queries for checking that player and match belong to the same team. That means two round trips to the database. You could reduce this by half if you use just one query e.g. given the following query:

        SELECT COUNT(*)
          FROM players AS p
    INNER JOIN matches AS m
            ON p.team_id = m.team_id
         WHERE p.id = NEW.player_id AND m.id = NEW.match_id
    

    you would change your function as follows:

    def validate_match_player(changeset) do
      player_id = get_field(changeset, :player_id)
      match_id = get_field(changeset, :match_id)
    
      [result] =
        Player
        |> join(:inner, [p], m in Match, on: p.team_id == m.team_id)
        |> where([p, m], p.id == ^player_id and m.id == ^match_id)
        |> select([p, m], %{count: count(p.id)})
        |> Repo.all()
    
      case result do
        %{count: 0} ->
          add_error(changeset, :player, "does not belong to the same team as the match")
        _ ->
          changeset
      end
    end
    

    Database Level Solution

    I'm assuming you're using PostgreSQL, so my answer will correspond to what you can find in the PostgreSQL manual.

    There's no (clean) way to define a constraint in the table that does this. Constraints can only access the table where they're defined. Some constraints can only access the column from what they're defined and nothing more (CHECK CONSTRAINT).

    The best approach would be writing a trigger for validating both fields e.g:

    CREATE OR REPLACE FUNCTION trigger_validate_match_player()
      RETURNS TRIGGER AS $$
        IF (
             SELECT COUNT(*)
             FROM players AS p
             INNER JOIN matches AS m
             ON p.team_id = m.team_id
             WHERE p.id = NEW.player_id AND m.id = NEW.match_id
           ) = 0
        THEN
          RAISE 'does not belong to the same team as the match'
            USING ERRCODE 'invalid_match_player';
        END IF;
    
        RETURN NEW;
      $$ LANGUAGE plpgsql;
    
    CREATE TRIGGER responses_validate_match_player
      BEFORE INSERT OR UPDATE ON responses
      FOR EACH ROW
      EXECUTE PROCEDURE trigger_validate_match_player();
    

    The previous trigger will raise an exception when it fails. This also means Ecto will raise an exception. You can see how to handle this exception here.

    In the end, maintaining triggers is not easy unless you're using something like sqitch for database migrations.

    PS: If you're curious, the very dirty way of doing this in a CHECK constraint is by defining a PostgreSQL function that basically bypasses the limitation. I wouldn't recommend it.

    I hope this helps :)