Merge pull request #2624 from ebean-orm/feature/2623-CurrentTenantPro…

…vider

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

ebean-core/src/main/java/io/ebeaninternal/server/core/MultiTenantDbCatalogSupplier.java

@@ -17,12 +17,14 @@
  */
 final class MultiTenantDbCatalogSupplier implements DataSourceSupplier {
 
+ private final CurrentTenantProvider tenantProvider;
  private final DataSource dataSource;
  private final DataSource readOnlyDataSource;
  private final CatalogDataSource catalogDataSource;
  private final CatalogDataSource readOnly;
 
  MultiTenantDbCatalogSupplier(CurrentTenantProvider tenantProvider, DataSource dataSource, DataSource readOnlyDataSource, TenantCatalogProvider catalogProvider) {
+ this.tenantProvider = tenantProvider;
  this.dataSource = dataSource;
  this.readOnlyDataSource = readOnlyDataSource;
  this.catalogDataSource = new CatalogDataSource(dataSource, tenantProvider, catalogProvider);
@@ -33,6 +35,11 @@ final class MultiTenantDbCatalogSupplier implements DataSourceSupplier {
  }
  }
 
+ @Override
+ public Object currentTenantId() {
+ return tenantProvider.currentId();
+ }
+
  @Override
  public DataSource getDataSource() {
  return catalogDataSource;

ebean-core/src/main/java/io/ebeaninternal/server/core/MultiTenantDbSchemaSupplier.java

@@ -17,12 +17,14 @@
  */
 final class MultiTenantDbSchemaSupplier implements DataSourceSupplier {
 
+ private final CurrentTenantProvider tenantProvider;
  private final DataSource dataSource;
  private final DataSource readOnlyDataSource;
  private final SchemaDataSource schemaDataSource;
  private final SchemaDataSource readOnly;
 
  MultiTenantDbSchemaSupplier(CurrentTenantProvider tenantProvider, DataSource dataSource, DataSource readOnlyDataSource, TenantSchemaProvider schemaProvider) {
+ this.tenantProvider = tenantProvider;
  this.dataSource = dataSource;
  this.readOnlyDataSource = readOnlyDataSource;
  this.schemaDataSource = new SchemaDataSource(dataSource, schemaProvider, tenantProvider);
@@ -33,6 +35,11 @@ final class MultiTenantDbSchemaSupplier implements DataSourceSupplier {
  }
  }
 
+ @Override
+ public Object currentTenantId() {
+ return tenantProvider.currentId();
+ }
+
  @Override
  public DataSource getDataSource() {
  return schemaDataSource;

ebean-core/src/main/java/io/ebeaninternal/server/core/MultiTenantDbSupplier.java

@@ -21,6 +21,11 @@ final class MultiTenantDbSupplier implements DataSourceSupplier {
  this.dataSourceProvider = dataSourceProvider;
  }
 
+ @Override
+ public Object currentTenantId() {
+ return tenantProvider.currentId();
+ }
+
  @Override
  public DataSource getReadOnlyDataSource() {
  // read only datasource not supported with DB per tenant at this stage

ebean-core/src/main/java/io/ebeaninternal/server/core/SimpleDataSourceProvider.java

@@ -20,6 +20,11 @@ final class SimpleDataSourceProvider implements DataSourceSupplier {
  this.readOnlyDataSource = readOnlyDataSource;
  }
 
+ @Override
+ public Object currentTenantId() {
+ return null; // not required here
+ }
+
  @Override
  public DataSource getDataSource() {
  return dataSource;

ebean-core/src/main/java/io/ebeaninternal/server/transaction/DataSourceSupplier.java

@@ -16,7 +16,6 @@ public interface DataSourceSupplier {
  * Return the DataSource to use for the current request.
  * <p>
  * This should take into account multi-tenancy and the current tenantId.
- * </p>
  */
  DataSource getDataSource();
 
@@ -25,10 +24,14 @@ public interface DataSourceSupplier {
  * <p>
  * This can return null meaning that no read only DataSource (with autoCommit)
  * is available for use so normal transactions with explicit commit should be used.
- * </p>
  */
  DataSource getReadOnlyDataSource();
 
+ /**
+ * Obtain the current TenantId *IF* it is required for the DataSource.
+ */
+ Object currentTenantId();
+
  /**
  * Return a connection from the DataSource taking into account a tenantId for multi-tenant lazy loading.
  *

ebean-core/src/main/java/io/ebeaninternal/server/transaction/TransactionFactoryTenantWithRead.java

@@ -27,8 +27,8 @@ public SpiTransaction createReadOnlyTransaction(Object tenantId) {
  Connection connection = null;
  try {
  if (tenantId == null) {
- // tenantId not set (by lazy loading) so get current tenantId
- tenantId = tenantProvider.currentId();
+ // obtain the tenantId if the DataSource requires it
+ tenantId = dataSourceSupplier.currentTenantId();
  }
  connection = dataSourceSupplier.getReadOnlyConnection(tenantId);
  return new ImplicitReadOnlyTransaction(manager, connection, tenantId);

ebean-test/src/test/java/org/multitenant/partition/CurrentTenant.java

@@ -2,13 +2,22 @@
 
 import io.ebean.config.CurrentTenantProvider;
 
+import java.util.concurrent.atomic.AtomicInteger;
+
 class CurrentTenant implements CurrentTenantProvider {
 
+ static AtomicInteger callCounter = new AtomicInteger();
+
+ static int count() {
+ return callCounter.get();
+ }
+
  /**
  * Return the current tenantId from the user context.
  */
  @Override
  public String currentId() {
+ callCounter.incrementAndGet();
  return UserContext.get().getTenantId();
  }
 }

ebean-test/src/test/java/org/multitenant/partition/MtNone.java

@@ -0,0 +1,21 @@
+package org.multitenant.partition;
+
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.Version;
+
+@Entity
+public class MtNone {
+
+ @Id
+ long id;
+
+ final String none;
+
+ @Version
+ long version;
+
+ public MtNone(String none) {
+ this.none = none;
+ }
+}

ebean-test/src/test/java/org/multitenant/partition/MultiTenantPartitionTest.java

@@ -1,11 +1,11 @@
 package org.multitenant.partition;
 
-import io.ebean.xtest.BaseTestCase;
 import io.ebean.Database;
 import io.ebean.DatabaseFactory;
 import io.ebean.config.DatabaseConfig;
 import io.ebean.config.TenantMode;
 import io.ebean.test.LoggedSql;
+import io.ebean.xtest.BaseTestCase;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Test;
 
@@ -15,7 +15,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-public class MultiTenantPartitionTest extends BaseTestCase {
+class MultiTenantPartitionTest extends BaseTestCase {
 
  private static final String[] names = {"Ace", "Base", "Case", "Dae", "Eva"};
 
@@ -33,13 +33,26 @@ static List<MtTenant> tenants() {
  }
 
  @AfterAll
- public static void shutdown() {
+ static void shutdown() {
  server.shutdown();
  }
 
  @Test
- public void start() {
+ void queryOnly_noTenantIdRequired_expect_currentTenantNotCalled() {
+ // MtNone does not have any @TenantId property
+ MtNone none = new MtNone("none");
+ server.save(none);
+
+ int beforeCount = CurrentTenant.count();
+ // CurrentTenant should not be called for this query
+ server.find(MtNone.class).findList();
+ int afterCount = CurrentTenant.count();
 
+ assertThat(afterCount).isSameAs(beforeCount);
+ }
+
+ @Test
+ void start() {
  UserContext.set("rob", "ten_1");
 
  LoggedSql.start();
@@ -64,8 +77,7 @@ public void start() {
  }
 
  @Test
- public void deleteById() {
-
+ void deleteById() {
  UserContext.set("fred", "ten_2");
 
  MtContent content = new MtContent("first title");
@@ -83,8 +95,7 @@ public void deleteById() {
  }
 
  @Test
- public void deleteByIds() {
-
+ void deleteByIds() {
  UserContext.set("fred", "ten_2");
 
  MtContent a = newContent("title a");
@@ -107,11 +118,8 @@ private MtContent newContent(String title) {
  return content;
  }
 
-
  private static Database init() {
-
  DatabaseConfig config = new DatabaseConfig();
-
  config.setName("h2multitenant");
  config.loadFromProperties();
  config.setDdlGenerate(true);
@@ -124,6 +132,7 @@ private static Database init() {
 
  config.getClasses().add(MtTenant.class);
  config.getClasses().add(MtContent.class);
+ config.getClasses().add(MtNone.class);
 
  return DatabaseFactory.create(config);
  }