Can't update tenant outside the public schema.

[bernardopires]

Can't update tenant outside the public schema.

[bernardopires]

I'm not sure if that is a good idea... calling set_schema_to_public()
will permanently (until the end of the request) set the search_path to
public, which may lead to other weird unexpected exceptions. This
exception is there exactly to make sure you're creating tenants from the
right place.

The problem you are talking about should not happen outside the shell. If
this code is inside a real view, after a tenant is created on public, the
schema gets automatically set back to public.

2013/8/27 Walkman [email protected]

There is a check if current schema is public in models.py:

https://github.com/bcarneiro/django-tenant-schemas/blob/master/tenant_schemas/models.py#L30

I wonder what is the purpose of this check?

Is it safe to simply:

if connection.get_schema() != get_public_schema_name():
connection.set_schema_to_public()

The problem is when I create a new tenant from shell, it always set the
same to the schema for that Tenant, but doesn't change it back, so I cannot
create a new Tenant from that shell session. Is it ok to simply set schema
to public?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/62
.

Bernardo Pires Carneiro

[kissgyorgy]

Hey @bcarneiro, I appreciate your fast responses! 👍
Why is that a problem? When I first start the shell and run a connection.get_schema() I got public, so are you saying creating Tenants from shell might be problematic? What could go wrong?
Also, Tenant instances saved only to the public schema, right? So when I want to addd a bunch of Tenants from shell, all I need is the public schema. I don't see any statements which require the tenant schema, when dealing with only the Tenant instance, or are there?
Or do you mean the problem is that public will set the rest of the shell session? Adding connection.set_schema(self.schema_name) at the end of the TenantModel save method will not solve this problem?

[bernardopires]

Hi Walkman,

Thank you for your feedback! When I wrote this project, I supposed people
would create tenants via normal django views. For example, the user goes to
a sign up page (which is on your public domain), fills his data in and at
the end a tenant gets created with the data he provided. The exception is
there just to protect you from trying to create a new tenant inside a
tenant's domain. This case should actually never happen, which is why it
throws an exception.

I thought that after creating a tenant, the search path was set back to
public, but after the checking the code I don't think this is happening.
This is exactly the problem you are having, right? I'd love if you could
submit a patch that calls set_schema_to_public AFTER the tenant is created.
This should also solve your problem, correct?

Cheers,
Bernardo

2013/8/28 Walkman [email protected]

Hey @bcarneiro https://github.com/bcarneiro, I appreciate your fast
responses! [image: 👍]
Why is that a problem? When I first start the shell and run a
connection.get_schema() I got public, so are you saying creating Tenants
from shell might be problematic? What could go wrong?
Also, Tenant instances saved only to the public schema, right? So when I
want to addd a bunch of Tenants from shell, all I need is the public
schema. I don't see any statements which require the tenant schema, when
dealing with only the Tenant instance, or are there?
Or do you mean the problem is that public will set the rest of the shell
session? Adding connection.set_schema(self.schema_name) at the end of the
TenantModel save method will not solve this problem?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/62#issuecomment-23407176
.

Bernardo Pires Carneiro

[kissgyorgy]

I thought that after creating a tenant, the search path was set back to
public, but after the checking the code I don't think this is happening.

Yes, after the tenants is created, search path stays tenant's schema only, public not included and current_schema is also tenant's schema.

This is exactly the problem you are having, right? This should also solve your problem, correct?

Yes, thank you!

I investigated further, and now that I understand what's going on, I'm pretty confident that on tenant's save() it's safe to do:

if connection.get_schema() != get_public_schema_name():
    connection.set_schema_to_public()

because here is what happens (please correct me if I'm wrong):

1. sync_schemas command set the schema and tenant properties on connection (by invoking connection.set_tenant())
2. then it pass other parameters to syncdb command
3. which gets cursor very early on, and because tenant_schemas/postgresql_backend/base.py modifies cursor with a call to set_search_path(), search path will be properly set before any other db operation begins, so syncing can happen without problems on the right SEARCH_PATH set.

Please confirm and I send the patch.

[bernardopires]

Hi Walkman,

it is safe to call connection.set_schema_to_public() where you want, but only in the sense that the code will work. The thing is, nobody should ever try to create a tenant being inside another tenant's domain, is this clear to you? This is why I don't want to force the schema to public before creating the tenant. I think the correct solution here is to call connection.set_schema_to_public() AFTER the tenant's creation (~line 73 on models.py, after migrations are called). Do you agree?

[kissgyorgy]

The thing is, nobody should ever try to create a tenant being inside another tenant's domain, is this clear to you?

Now, I understand what you wanted to achieve with this :D, but this way, nobody is able to modify their own tenant (for example name, related members, etc) from their own domain right now, which would be more convenient if they could so and I don't have to redirect them to public website just for a small modification.
What you think about this:

if connection.get_schema() != self.schema_name and connection.get_schema() != get_public_schema_name():
    raise Exception("Can't update tenant outside the public schema. Current schema is %s." % coonnection.get_schema())

I think the correct solution here is to call connection.set_schema_to_public() AFTER the tenant's creation (~line 73 on models.py, after migrations are called). Do you agree?

I agree that this is the right place, but to comply your expectations, it should rather set the previous schema before the sync, rather than public.
something like this:

def save(self, verbosity=1, *args, **kwargs):
    if connection.get_schema() != self.schema_name and connection.get_schema() != get_public_schema_name():
        raise Exception("Can't update tenant outside the public schema. Current schema is %s."
                        % connection.get_schema())
    else:
        self.current_schema = connection.get_schema()

def create_schema(self, check_if_exists=False, sync_schema=True, verbosity=1):
    # CREATE SCHEMA
    # SYNC SCHEMA
    # MIGRATE SCHEMA

    connection.set_schema(self.current_schema)
    return True

Or maybe it would be better to connection.set_tenant(self)

What you think?

[bernardopires]

Hey Walkman,

sorry for taking so long to reply. Alright, alright, I've changed my mind and I have to agree with you! I also had to update a tenant from inside the tenant's domain and my own code bit me too. It does make sense to allow a tenant to update itself. I also like the solution you presented! I'd only change some details.

I'd do something like this on save

    def save(self, verbosity=1, *args, **kwargs):
        is_new = self.pk is None

        if is_new and connection.get_schema() != get_public_schema_name():
            raise Exception("Can't create tenant outside the public schema. Current schema is %s."
                            % connection.get_schema())

        if not is_new and connection.get_schema() not in (self.schema_name, get_public_schema_name()):
            raise Exception("Can't update tenant outside it's own schema or the public schema. Current 
            schema is %s." % connection.get_schema())

        super(TenantMixin, self).save(*args, **kwargs)

        if is_new and self.auto_create_schema:
            self.create_schema(check_if_exists=True, verbosity=verbosity)
            post_schema_sync.send(sender=TenantMixin, tenant=self)

        transaction.commit_unless_managed()

On create_schema you can safely call connection.set_schema_to_public() at the end, because you can only create tenants from the public schema.

Are you happy with this solution? Thanks for the help and the discussion!

Cheers,
Bernardo

[kissgyorgy]

At the end of create_schema, we should set the active schema before create_schema.
Imagine this situation:
You create a new Tenant. Syncdb command set the schema to that tenant's schema. Schema changes to public at the end of create_schema. You want to update some other models, but it updates the public schema only, because the search path is now set to public!

    def save(self, verbosity=1, *args, **kwargs):
        is_new = self.pk is None

        if is_new and connection.get_schema() != get_public_schema_name():
            raise Exception("Can't create tenant outside the public schema. Current schema is %s."
                            % connection.get_schema())

        if not is_new and connection.get_schema() not in (self.schema_name, get_public_schema_name()):
            raise Exception("Can't update tenant outside it's own schema or the public schema. Current 
            schema is %s." % connection.get_schema())

        self.current_schema = connection.get_schema()
        super(TenantMixin, self).save(*args, **kwargs)

        if is_new and self.auto_create_schema:
            self.create_schema(check_if_exists=True, verbosity=verbosity)
            post_schema_sync.send(sender=TenantMixin, tenant=self)

        transaction.commit_unless_managed()

    def create_schema(...):
       # CREATE, sync
       connection.set_schema(self.current_schema)

       return True

This way, if you invoke it from shell, starting schema will be public, and after creation, it will be public either, so we can create a new schema.

Any more thoughts? :D

[bernardopires]

I don't understand what you want to achieve? current_schema will always
be public before creating tenants, else an exception is thrown

2013/9/2 Walkman [email protected]

At the end of create_schema, we should set the active schema before
create_schema.
Imagine this situation:
You create a new Tenant. Syncdb command set the schema to that tenant's
schema. Schema changes to public at the end of create_schema. You want to
update some other models, but it updates the public schema only, because
the search path is now set to public!
This way, if you invoke it from shell, starting schema will be public, and
after creation, it will be public either, so we can create a new schema.

def save(self, verbosity=1, *args, **kwargs):
    is_new = self.pk is None

    if is_new and connection.get_schema() != get_public_schema_name():
        raise Exception("Can't create tenant outside the public schema. Current schema is %s."
                        % connection.get_schema())

    if not is_new and connection.get_schema() not in (self.schema_name, get_public_schema_name()):
        raise Exception("Can't update tenant outside it's own schema or the public schema. Current
        schema is %s." % connection.get_schema())

    self.current_schema = connection.get_schema()
    super(TenantMixin, self).save(*args, **kwargs)

    if is_new and self.auto_create_schema:
        self.create_schema(check_if_exists=True, verbosity=verbosity)
        post_schema_sync.send(sender=TenantMixin, tenant=self)

    transaction.commit_unless_managed()

def create_schema(...):
   # CREATE, sync
   connection.set_schema(self.current_schema)

Any more thoughts? :D

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/62#issuecomment-23666759
.

Bernardo Pires Carneiro

[kissgyorgy]

You are right, that was a stupid question by me. :)