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

CurrentTenantProvider called even when table has no tenantId column specified #2623

Closed
Brian-Hofmann opened this issue Mar 31, 2022 · 4 comments · Fixed by #2624
Closed

CurrentTenantProvider called even when table has no tenantId column specified #2623

Brian-Hofmann opened this issue Mar 31, 2022 · 4 comments · Fixed by #2624
Assignees
Labels
Milestone

Comments

@Brian-Hofmann
Copy link

Expected behavior

Based on the class structure below, I thought this query would not call the current tenant provider:

new QNoTenantEntity().where().name.eq(name).findOneOrEmpty();

Actual behavior

The current tenant provider is called

public class MyTenantProvider implements CurrentTenantProvider {
    @Override
    public String currentId() {
        return UserContext.get().getTenantId();
    }
}

Steps to reproduce

@MappedSuperclass
public class Base extends Model {

    @Id
    private long id;

    public long getId() {
        return id;
    }

    public void setId(long id) {
        this.id = id;
    }


}
@MappedSuperclass
public class Tenant extends Base {

    @TenantId
    private String tenantId;

    public String getTenantId() {
        return tenantId;
    }

    public void setTenantId(String tenantId) {
        this.tenantId = tenantId;
    }

}
@Entity
@Table(name = "no-tenant")
public class NoTenantEntity extends Base {

    @NotNull
    private String name;

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

}
@Entity
@Table(name = "user")
public class UserEntity extends Tenant {

    @NotNull
    private String firstName;

    public String getFirstName() {
        return firstName;
    }

    public void setFirstName(String firstName) {
        this.firstName = firstName;
    }

}

This is an issue since certain calls to my database are non-tenant specific based on the table they are accessing. I can get around it by setting a fake user context, but that seemed like a hacky answer since no logged in user is calling the database. Thank you in advance!

@rbygrave
Copy link
Member

Reproduced and got a fix in that linked PR. Effectively the query should only get the current tenantId IF we are using a DataSourceProvider that actually needs it (like DataSource per tenant, Schema per tenant etc).

In this case I'm 99% sure its TenantMode.PARTITION (aka tenant_id column in the tables). That means it's using the SimpleDataSourceProvider ... which does not need tenantId in order to get a [read only] connection for this case. So the fix here is to delegate to the DataSourceSupplier to get or not the current tenantId and SimpleDataSourceProvider doesn't need the tenantId.

Not sure if I'm explained that well, might be easier to look at the diff in the PR :)

@Brian-Hofmann
Copy link
Author

Brian-Hofmann commented Mar 31, 2022

Yes I understand! Thank you for getting back to me so quickly. I am using TenantMode.PARTITION, so you are correct.

One final question here on the topic. If the non-tenant entity were to reference an entity that had a tenant ID...for example:

@Entity
@Table(name = "no-tenant")
public class NoTenantEntity extends Base {

    @NotNull
    private String name;
    @OneToMany
    private List<UserEntity> users;

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }
    
    ...additional getters and setters for users....

}

Would a query getitng the non-tenant entity always return an empty list of users unless you call the non-tenant entity as a logged in user (thus having a tenant id to reference)?

@rbygrave
Copy link
Member

rbygrave commented Mar 31, 2022

Would a query getitng the non-tenant entity ...

A query for a non-tenant entity means that there is no tenant_id = ?predicate added to the where clause.

Reference: https:/ebean-orm/ebean/blob/master/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java#L1121 ... the tenant BeanProperty is null ... so tenant.addTenant(query, tenantId); is not called, no tenant_id = ? is added.

always return an empty list of users unless you ...

It would be a full list of related users.

So a query like:

new QNoTenantEntity()
.users.fetch() // fetch associated entities that happen to have a @TenantId property
.findList();

The query will execute without a tenant_id = ? predicate, it will return the top level no-tenant entity and it's users list will contain all the users (which can be for different tenants).

That is, in a multi-tenant app, often there are parts of the application are kind of "global" (not partitioned by tenant). When we query these non-tenant entities it is expected to be app code that is doing "global" things and not "per tenant" things. For example, some global admin function looking at all the users across all tenants.

rbygrave added a commit that referenced this issue Mar 31, 2022
…vider

#2623 - CurrentTenantProvider called even when table has no tenantId column specified
@Brian-Hofmann
Copy link
Author

That makes sense. Thank you!

@rbygrave rbygrave modified the milestones: 13.0.1, 13.1.0 Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants