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

Alter the query instead of the repo #21

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jeffdeville
Copy link

I've been struggling w/ other libraries that are expecting an object with Ecto's standard method signatures. This strategy leaves the existing functionality untouched, but exposes the private helper methods in their own module so they can be pulled in separately.

A few points to note:

  • add_prefix_to_query and add_prefix were both renamed to just set_tenant to keep the api easier. If there is an important reason why that might be a bad thing, please let me know. But I did alter the version targeting Ecto models to look for meta instead of struct. It seemed to work w/out that change, but I'm not sure why.
  • If you're ok w/ this change, I can append this strategy to the docs.

@jeffdeville
Copy link
Author

jeffdeville commented Sep 9, 2016

Disregard this comment, it's all now in the PR
A few other thoughts based on work I'm doing in my project:

I would like to enforce the specification of a tenant, and raise an appropriate error if one is missing. To do this, I've created a repo that wraps the 'real' ecto repo. It will then check for the prefix and raise if it is missing. Other methods are then defdelegate-d to the repo.

I'm not thrilled with how this looks. I figure there is probably a metaprogramming solution that will do this much more elegantly, and without me having to be so intimately tied to the stuff being generated by Ecto. But so far it's working.

defmodule RestConnectder.ApartmentexRepo.TenantMissingError do
  defexception message: "No tenant specified"
end

defmodule RestConnectder.ApartmentexRepo do
  alias RestConnectder.ApartmentexRepo.TenantMissingError
  alias RestConnectder.Repo
  alias Ecto.Changeset

  defdelegate __adapter__, to: Repo
  defdelegate __log__(entry), to: Repo
  defdelegate config(), to: Repo
  defdelegate start_link(opts \\ []), to: Repo
  defdelegate stop(pid, timeout \\ 5000), to: Repo
  defdelegate transaction(fun_or_multi, opts \\ []), to: Repo
  defdelegate in_transaction?(), to: Repo
  defdelegate rollback(value), to: Repo
  defdelegate aggregate(queryable, aggregate, field, opts \\ []), to: Repo
  defdelegate preload(struct_or_structs, preloads, opts \\ []), to: Repo
  defdelegate __pool__, to: Repo
  defdelegate __sql__, to: Repo

  def all(queryable, opts \\ []) do
    assert_tenant(queryable)
    Repo.all(queryable, opts)
  end

  def get(queryable, id, opts \\ []) do
    assert_tenant(queryable)
    Repo.get(queryable, id, opts)
  end

  def get!(queryable, id, opts \\ []) do
    assert_tenant(queryable)
    Repo.get!(queryable, id, opts)
  end

  def get_by(queryable, clauses, opts \\ []) do
    assert_tenant(queryable)
    Repo.get_by(queryable, clauses, opts)
  end

  def get_by!(queryable, clauses, opts \\ []) do
    assert_tenant(queryable)
    Repo.get_by!(queryable, clauses, opts)
  end

  def one(queryable, opts \\ []) do
    assert_tenant(queryable)
    Repo.one(queryable, opts)
  end

  def one!(queryable, opts \\ []) do
    assert_tenant(queryable)
    Repo.one!(queryable, opts)
  end

  def insert_all(schema_or_source, entries, opts \\ []) do
    assert_tenant(schema_or_source)
    Repo.insert_all(schema_or_source, entries, opts)
  end

  def update_all(queryable, updates, opts \\ []) do
    assert_tenant(queryable)
    Repo.update_all(queryable, updates, opts)
  end

  def delete_all(queryable, opts \\ []) do
    assert_tenant(queryable)
    Repo.delete_all(queryable, opts)
  end

  def insert(struct, opts \\ []) do
    assert_tenant(struct)
    Repo.insert(struct, opts)
  end

  def update(struct, opts \\ []) do
    assert_tenant(struct)
    Repo.update(struct, opts)
  end

  def insert_or_update(changeset, opts \\ []) do
    assert_tenant(changeset)
    Repo.insert_or_update(changeset, opts)
  end

  def delete(struct, opts \\ []) do
    assert_tenant(struct)
    Repo.delete(struct, opts)
  end

  def insert!(struct, opts \\ []) do
    assert_tenant(struct)
    Repo.insert!(struct, opts)
  end

  def update!(struct, opts \\ []) do
    assert_tenant(struct)
    Repo.update!(struct, opts)
  end

  def insert_or_update!(changeset, opts \\ []) do
    assert_tenant(changeset)
    Repo.insert_or_update!(changeset, opts)
  end

  def delete!(struct, opts \\ []) do
    assert_tenant(struct)
    Repo.delete!(struct, opts)
  end

  def assert_tenant(%Changeset{} = changeset) do
    assert_tenant(changeset.data)
  end

  def assert_tenant(%{__meta__: _} = model) do
    case Ecto.get_meta(model, :prefix) do
      nil -> raise TenantMissingError
      _ -> nil
    end
  end

  def assert_tenant(queryable) do
    case queryable |> Ecto.Queryable.to_query |> Map.fetch(:prefix) do
    {:ok, _} -> nil
    {:error} -> raise TenantMissingError
    end
  end
end

In my case, all of the tables are in schemas except for the tenants themselves, so when I create a tenant, I pass in the 'real' Repo. But ideally, this could be configured in such a way that you could tell it which tables require tenants, and which do not.

Thoughts/Suggestions?

@jeffdeville
Copy link
Author

jeffdeville commented Sep 9, 2016

Disregard this comment, it's all now in the PR
Sorry, here's a slightly improved macro solution:

defmodule RestConnectder.RepoDelegation do
  defmacro __using__(opts) do
    quote bind_quoted: [opts: opts] do
      @repo Keyword.fetch!(opts, :repo)
      # @untenanted_models Keyword.get(opts, :repo, [])

      # From Ecto.Repo
      defdelegate __adapter__, to: @repo
      defdelegate __log__(entry), to: @repo
      defdelegate config(), to: @repo
      defdelegate start_link(opts \\ []), to: @repo
      defdelegate stop(pid, timeout \\ 5000), to: @repo
      defdelegate transaction(fun_or_multi, opts \\ []), to: @repo
      defdelegate in_transaction?(), to: @repo
      defdelegate rollback(value), to: @repo
      defdelegate aggregate(queryable, aggregate, field, opts \\ []), to: @repo
      defdelegate preload(struct_or_structs, preloads, opts \\ []), to: @repo

      # From Ecto.Adapters.SQL
      defdelegate __pool__, to: @repo
      defdelegate __sql__, to: @repo

      def assert_tenant(%Ecto.Changeset{} = changeset) do
        assert_tenant(changeset.data)
      end

      def assert_tenant(%{__meta__: _} = model) do        
        case Ecto.get_meta(model, :prefix) do
          nil -> raise RestConnectder.ApartmentexRepo.TenantMissingError
          _ -> nil
        end
      end

      def assert_tenant(queryable) do
        case queryable |> Ecto.Queryable.to_query |> Map.fetch(:prefix) do
        {:ok, _} -> nil
        {:error} -> raise RestConnectder.ApartmentexRepo.TenantMissingError
        end
      end
    end
  end
end

defmodule RestConnectder.ApartmentexRepo do
  @behaviour Ecto.Repo
  use RestConnectder.RepoDelegation, repo: RestConnectder.Repo #, untenanted_models: [RestConnectder.Company]
  alias RestConnectder.Repo
  alias Ecto.Changeset

  def all(queryable, opts \\ []) do
    assert_tenant(queryable)
    Repo.all(queryable, opts)
  end

  def get(queryable, id, opts \\ []) do
    assert_tenant(queryable)
    Repo.get(queryable, id, opts)
  end

  def get!(queryable, id, opts \\ []) do
    assert_tenant(queryable)
    Repo.get!(queryable, id, opts)
  end

  def get_by(queryable, clauses, opts \\ []) do
    assert_tenant(queryable)
    Repo.get_by(queryable, clauses, opts)
  end

  def get_by!(queryable, clauses, opts \\ []) do
    assert_tenant(queryable)
    Repo.get_by!(queryable, clauses, opts)
  end

  def one(queryable, opts \\ []) do
    assert_tenant(queryable)
    Repo.one(queryable, opts)
  end

  def one!(queryable, opts \\ []) do
    assert_tenant(queryable)
    Repo.one!(queryable, opts)
  end

  def insert_all(schema_or_source, entries, opts \\ []) do
    assert_tenant(schema_or_source)
    Repo.insert_all(schema_or_source, entries, opts)
  end

  def update_all(queryable, updates, opts \\ []) do
    assert_tenant(queryable)
    Repo.update_all(queryable, updates, opts)
  end

  def delete_all(queryable, opts \\ []) do
    assert_tenant(queryable)
    Repo.delete_all(queryable, opts)
  end

  def insert(struct, opts \\ []) do
    assert_tenant(struct)
    Repo.insert(struct, opts)
  end

  def update(struct, opts \\ []) do
    assert_tenant(struct)
    Repo.update(struct, opts)
  end

  def insert_or_update(changeset, opts \\ []) do
    assert_tenant(changeset)
    Repo.insert_or_update(changeset, opts)
  end

  def delete(struct, opts \\ []) do
    assert_tenant(struct)
    Repo.delete(struct, opts)
  end

  def insert!(struct, opts \\ []) do
    assert_tenant(struct)
    Repo.insert!(struct, opts)
  end

  def update!(struct, opts \\ []) do
    assert_tenant(struct)
    Repo.update!(struct, opts)
  end

  def insert_or_update!(changeset, opts \\ []) do
    assert_tenant(changeset)
    Repo.insert_or_update!(changeset, opts)
  end

  def delete!(struct, opts \\ []) do
    assert_tenant(struct)
    Repo.delete!(struct, opts)
  end
end

once you can pass in the models that should and should not be tenanted, I think this entire thing can be simplified to just:

defmodule RestConnectder.ApartmentexRepo do
  @behaviour Ecto.Repo
  use RestConnectder.RepoDelegation, repo: RestConnectder.Repo, untenanted_models: [RestConnectder.Company]

@Dania02525
Copy link
Owner

Wondering if we can just import Ecto.Repo, then do something like enumerating through info(:functions) to patch the functions with the guard. That or proxy module that checks the guard before passing function/arity direct to Ecto.Repo. I'm on a road trip, but will be back Monday to take a look :-)

@jeffdeville
Copy link
Author

jeffdeville commented Sep 9, 2016

@Dania02525 I've got this sorted out, I think. No rush. Enjoy your road trip!

@jeffdeville jeffdeville force-pushed the alternative_tenant_specification branch from 8c91ec6 to bad33fe Compare September 9, 2016 18:38
@jeffdeville
Copy link
Author

Hi @Dania02525, have you had a chance to look into this at all? I tried the 'enumerating' option you were thinking first, but in the end I skipped it because:

  1. the opts \ [] means that each overridden function would have mean 2 methods created (all you get is delete!\3, delete!\2 from the info(:functions) call
  2. Not all of the methods are being overridden, which means I'd need to maintain a list of which were just delegated, and which got the set_tenant call. That impairs the code's ability to be resilient to Ecto changes anyway, so I figured I may as well make it easier to read and debug

@danielfoxp2
Copy link

I was just starting to work on a pull request to swap the repo and the queryable places because repo as first param blocks apartmentex api of being piped. But your solution do the job too.

@jeffdeville
Copy link
Author

@danielfoxp2 - https://github.com/promptworks/tenantex is what I wound up with.

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

Successfully merging this pull request may close these issues.

3 participants