#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 @@ import java.util.logging.Logger;
  */
 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 @@ import java.util.logging.Logger;
  */
 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 @@ final class TransactionFactoryTenantWithRead extends TransactionFactoryTenant {
     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 @@ package org.multitenant.partition;
 
 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 java.util.List;
 
 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 @@ public class MultiTenantPartitionTest extends BaseTestCase {
   }
 
   @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 class MultiTenantPartitionTest extends BaseTestCase {
   }
 
   @Test
-  public void deleteById() {
-
+  void deleteById() {
     UserContext.set("fred", "ten_2");
 
     MtContent content = new MtContent("first title");
@@ -83,8 +95,7 @@ public class MultiTenantPartitionTest extends BaseTestCase {
   }
 
   @Test
-  public void deleteByIds() {
-
+  void deleteByIds() {
     UserContext.set("fred", "ten_2");
 
     MtContent a = newContent("title a");
@@ -107,11 +118,8 @@ public class MultiTenantPartitionTest extends BaseTestCase {
     return content;
   }
 
-
   private static Database init() {
-
     DatabaseConfig config = new DatabaseConfig();
-
     config.setName("h2multitenant");
     config.loadFromProperties();
     config.setDdlGenerate(true);
@@ -124,6 +132,7 @@ public class MultiTenantPartitionTest extends BaseTestCase {
 
     config.getClasses().add(MtTenant.class);
     config.getClasses().add(MtContent.class);
+    config.getClasses().add(MtNone.class);
 
     return DatabaseFactory.create(config);
   }