influitive / apartment

Database multi-tenancy for Rack (and Rails) applications
2.67k stars 463 forks source link

Relations between public and tenant models bad design? #579

Open cdekker opened 5 years ago

cdekker commented 5 years ago

Would you consider it bad design to have a model in the public schema have a relation with a model in the private or tenant schema?

Especially the first example is giving me issues. Since rails 5.2 (?) belongs_to associations are required by default, meaning that unless the correct tenant is selected, a public record cannot be validated (since the referenced record will not be found)

Is this bad design? Or are there better ways to solve it?

I have a User model that I need to be in the public schema because this is how I log in and select the right tenant. This User model is dependent on an 'access scope' model that I need to be in the tenant schema. I am running into issue that all my User records are invalid, because it cannot find this access scope relation.

Is moving the accessScope` model into the public schema the only right way to solve this?

mikecmpbll commented 5 years ago

the example you outlined should work fine. you can't do regular joins on the association, obviously, but @user.access_scopes should just run a query like

select access_scopes.* from access_scopes where user_id = ?

against the tenant schema, which shouldn't be a problem.

I am running into issue that all my User records are invalid, because it cannot find this access scope relation.

can you look at which queries are being executed during User validation?

rayancastro commented 5 years ago

I have the same issue here, where a Post at a tenant schema, belongs to a user in the public schema. The Error and Query i get, when i'm trying to update my post, are:

ActiveRecord::InvalidForeignKey: PG::ForeignKeyViolation: ERROR: insert or update on table "posts" violates foreign key constraint "fk_rails_04d13ef8c7" DETAIL: Key (author_id)=(1) is not present in table "users". : UPDATE "posts" SET "updated_at" = $1, "author_id" = $2 WHERE "posts"."id" = $3 from /home/rayan/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/activerecord-5.2.2.1/lib/active_record/connection_adapters/postgresql_adapter.rb:611:inasync_exec' Caused by PG::ForeignKeyViolation: ERROR: insert or update on table "posts" violates foreign key constraint "fk_rails_04d13ef8c7" DETAIL: Key (author_id)=(1) is not present in table "users". `

Apparently, it does not find the table users, because its looking for it in the current schema, instead of the public schema

capjuancode commented 5 years ago

What if you exclude that model that you want to use in public?

psantos10 commented 4 years ago

I have the same issue described by @rayancastro

and as @capjuancode suggested, I also have the following line at config/initializers/apartment.rb

  config.excluded_models = %w[Account User Membership Invitation]

But I still have the error:

ActiveRecord::InvalidForeignKey (PG::ForeignKeyViolation: ERROR: insert or update on table "votes" violates foreign key constraint "fk_rails_c9b3bef597" DETAIL: Key (user_id)=(2) is not present in table "users". : INSERT INTO "votes" ("question_id", "option_id", "user_id", "created_at", "updated_at") VALUES ($1, $2, $3, $4, $5) RETURNING "id"):

Any idea how to solve this?

dalezak commented 4 years ago

@psantos10 looks like a workaround to avoid PG::ForeignKeyViolation is to remove foreign_key: true from your migration.

https://github.com/influitive/apartment/issues/382#issuecomment-306508376

Remove foreign_key: true from the migration file.

https://github.com/influitive/apartment/issues/248#issuecomment-129428464

There's currently no support for foreign keys across tenants. The 'solution' (unfortunately) is to just remove those foreign key constraints. Everything else will work as expected.

Hussainbhatti1 commented 4 years ago

Remove old foreign-key relation remove_foreign_key :employees, :restaurants And create a new one with public schema like this add_foreign_key :employees, 'public.restaurants', column: :restaurant_id, on_delete: :cascade

dalezak commented 4 years ago

@Hussainbhatti1 I tried your suggestion however in my case adding

add_foreign_key :meetings, 'public.users', column: :user_id, on_delete: :cascade

Simply adds the following to schema.rb

add_foreign_key "meetings", "users", on_delete: :cascade

Which still causes the PG::ForeignKeyViolation error

PG::ForeignKeyViolation: ERROR:  
insert or update on table "meetings" violates foreign key constraint "fk_rails_400671c98f" 
DETAIL:  Key (user_id)=(1) is not present in table "users".

Similar problem reported here https://github.com/influitive/apartment/issues/232#issuecomment-291288527

marksiemers commented 4 years ago

It seems the ability to have foreign keys from tenant databases to the public schema is a long requested feature.

It also sounds like removing the tables for excluded models from the tenant schemas could provide that feature support (at least for postgresql).

Would something like this be feasible?

  1. Provide a new config option, something like create_excluded_model_tables_in_tenant_schemas
    1. It defaults to true, supporting existing functionality
    2. Switching to false would not include those tables, indexes, or foreign keys when loading from schema.rb for tenant schemas
  2. Provide a method to put in migrations, something like skip_tenants!
    1. For migrations that change the excluded models, this method could be placed in the migration and would essentially do: return unless Apartment::Tenant.current == 'public'

There are some challenges here:

Perhaps when the option is set to false to enable this new functionality, when apartment loads, it can check all the things that would make the feature incompatible and raise an error.

At least in concept:

  1. Community - would this be helpful? Or are there too many limitations?
  2. Maintainers/Contributors - does this seem feasible? Difficult to maintain?