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

Missing multi-tenancy support #280

Closed
joeppeeters opened this issue Apr 10, 2018 · 9 comments
Closed

Missing multi-tenancy support #280

joeppeeters opened this issue Apr 10, 2018 · 9 comments

Comments

@joeppeeters
Copy link

Hi,

In our project we implement multi-tenancy by setting prefixes on the Repo (eg. MyApp.Repo.insert(%User{name: 'John'}, prefix: tenant_id)

We'd like to reuse this behaviour in our tests, but ex_machina doesn't seem to support it currently. Would that be something you would consider to add?

An easy way would be to create a factory for a test tenant by configure it in the use statement:

defmodule MyFactory do
  use ExMachina.Ecto, repo: MyRepo, repo_options: [prefix: "test_tenant"]

  ...
end

and then rewriting the ExMachina.EctoStrategy to:

...
def handle_insert(%{__meta__: %{__struct__: Ecto.Schema.Metadata}} = record, %{repo: repo, repo_options}) do
    record
    |> cast
    |> repo.insert!(repo_options)
  end
...

or more flexible at runtime by introducing an insert/3 method.

Is this something worth to consider? Or do you see any other ways to achieve the same behaviour?

@churcho
Copy link

churcho commented Feb 8, 2019

My team just hit this snag.
What options are there with regards to this?

@germsvel
Copy link
Collaborator

Thanks for posting here @churcho, and sorry @joeppeeters for taking so long to respond here! We currently don't have anything that supports multitenancy.

There is also a conversation on #273 (comment) that might be related, since they're also trying to provide extra arguments to Repo.insert!. I think the only option for now is to create a custom strategy that has those options defined in handle_insert. Naturally, that's not ideal because you can't really defined them at runtime like this,

build(:user) |> insert(prefix: "test_tenant")

You would have to define them in each strategy you create,

# in TestTenantEctoStrategy or something like that
def handle_insert(%{__meta__: %{__struct__: Ecto.Schema.Metadata}} = record, %{repo: repo}) do
    record
    |> cast
    |> repo.insert!(prefix: "test_tenant")
  end

Is that something that might work?

@nicocharlery
Copy link

nicocharlery commented Apr 9, 2019

Hi @germsvel
I have tried what you suggest with a custom strategy:

defmodule Datastore.TenantEctoStrategy do
  use ExMachina.Strategy, function_name: :insert
...
end

However, this leads to a conflict with the insert function defined within the library Ecto strategy:

== Compilation error in file test/support/factory.ex ==
** (CompileError) test/support/factory.ex:3: def insert/1 conflicts with defaults from insert/2
    test/support/factory.ex:3: (module)

However, it works with another function name.

@woolfred
Copy link

woolfred commented Jan 24, 2020

I added something like this to our factory 🤷‍♂

def tenant_insert(factory_name, attrs \\ %{}, tenant) do
    build(factory_name, attrs)
    |> MyApp.Repo.insert!(prefix: tenant)
end

We will see how far this will get us 😅

Edit: With the mentioned release v2.6.0 below this might look like:

def tenant_insert(factory_name, attrs \\ %{}, tenant) do
    insert(factory_name, attrs, prefix: tenant)
end

@hl
Copy link

hl commented Mar 31, 2020

The prefix option can also be set on the struct itself with Ecto.put_meta/2

def checkout_factory do
  %Checkout{
    (...)
  }
  |> Ecto.put_meta(prefix: "test")
end

@germsvel
Copy link
Collaborator

I was looking through this issue once again, and though I still think adding prefix options to insert during runtime could be difficult (given how ExMachina's strategies work), I think the original solution by @joeppeeters in would be a good first step.

It means we'd have to define a new factory per tenant, but maybe that would be good enough for some time? And if you need it per struct, @hl's comment on Ecto.put_meta/2 could be the trick.

What do people think? If it seems good to move with a use ExMachina.Ecto, repo: MyRepo, repo_options: [prefix: "test_tenant"], we can do that.

@mkarnebeek
Copy link

I'd very much prefer use ExMachina.Ecto, repo: MyRepo, repo_options: [prefix: "test_tenant"]. I've hit this snag a few times the past years and now again.

@germsvel
Copy link
Collaborator

germsvel commented Apr 2, 2020

Excellent! Would love a PR for this if someone is interested in doing that. (Or I'll try to get this in at some point)

It's been mentioned before, but just so there's no confusion, the usage would be to specify repo_options in the factory:

defmodule MyTestTenantFactory do
  use ExMachina.Ecto, repo: MyRepo, repo_options: [prefix: "test_tenant"]
end

That would apply globally to all factories defined in MyTestTenantFactory.

Does that sound good? 👍 👎

@germsvel
Copy link
Collaborator

I just pushed v2.6.0 of ExMachina which includes passing Repo options to insert/3. I know it's not a way to globally pass the prefix to all factories in a given file, but it might help with unblocking at least some of the work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants