From 50edc7a19ba6a36d5a45c429a6ca20d9ef0c8b02 Mon Sep 17 00:00:00 2001 From: Sattvik Chakravarthy Date: Mon, 15 Apr 2024 16:12:14 +0530 Subject: [PATCH 01/12] fix: core db crash --- .../storage/postgresql/ConnectionPool.java | 9 ++++-- .../supertokens/storage/postgresql/Start.java | 29 +++++++++++++++---- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/main/java/io/supertokens/storage/postgresql/ConnectionPool.java b/src/main/java/io/supertokens/storage/postgresql/ConnectionPool.java index d0b8c6f0..1f5d01a3 100644 --- a/src/main/java/io/supertokens/storage/postgresql/ConnectionPool.java +++ b/src/main/java/io/supertokens/storage/postgresql/ConnectionPool.java @@ -35,9 +35,11 @@ public class ConnectionPool extends ResourceDistributor.SingletonResource { private static final String RESOURCE_KEY = "io.supertokens.storage.postgresql.ConnectionPool"; private HikariDataSource hikariDataSource; private final Start start; + private Runnable postConnectCallback; - private ConnectionPool(Start start) { + private ConnectionPool(Start start, Runnable postConnectCallback) { this.start = start; + this.postConnectCallback = postConnectCallback; } private synchronized void initialiseHikariDataSource() throws SQLException { @@ -96,6 +98,7 @@ private synchronized void initialiseHikariDataSource() throws SQLException { config.setPoolName(start.getUserPoolId() + "~" + start.getConnectionPoolId()); try { hikariDataSource = new HikariDataSource(config); + this.postConnectCallback.run(); } catch (Exception e) { throw new SQLException(e); } @@ -133,7 +136,7 @@ static boolean isAlreadyInitialised(Start start) { return getInstance(start) != null && getInstance(start).hikariDataSource != null; } - static void initPool(Start start, boolean shouldWait) throws DbInitException { + static void initPool(Start start, boolean shouldWait, Runnable postConnectCallback) throws DbInitException { if (isAlreadyInitialised(start)) { return; } @@ -146,7 +149,7 @@ static void initPool(Start start, boolean shouldWait) throws DbInitException { " specified the correct values for ('postgresql_host' and 'postgresql_port') or for " + "'postgresql_connection_uri'"; try { - ConnectionPool con = new ConnectionPool(start); + ConnectionPool con = new ConnectionPool(start, postConnectCallback); start.getResourceDistributor().setResource(RESOURCE_KEY, con); while (true) { try { diff --git a/src/main/java/io/supertokens/storage/postgresql/Start.java b/src/main/java/io/supertokens/storage/postgresql/Start.java index 4e2ee20e..21ddd8ef 100644 --- a/src/main/java/io/supertokens/storage/postgresql/Start.java +++ b/src/main/java/io/supertokens/storage/postgresql/Start.java @@ -98,10 +98,7 @@ import java.sql.Connection; import java.sql.SQLException; import java.sql.SQLTransactionRollbackException; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Set; +import java.util.*; import static io.supertokens.storage.postgresql.QueryExecutorTemplate.execute; @@ -129,6 +126,7 @@ public class Start boolean enabled = true; static Thread mainThread = Thread.currentThread(); private Thread shutdownHook; + private Set tenantIdentifiers = new HashSet<>(); private boolean isBaseTenant = false; @@ -228,13 +226,32 @@ public void initStorage(boolean shouldWait) throws DbInitException { mainThread = Thread.currentThread(); } try { - ConnectionPool.initPool(this, shouldWait); - GeneralQueries.createTablesIfNotExists(this); + ConnectionPool.initPool(this, shouldWait, () -> { + try { + GeneralQueries.createTablesIfNotExists(this); + } catch (Exception e2) { + throw new IllegalStateException(e2); + } + for (TenantIdentifier tenantIdentifier : this.tenantIdentifiers) { + try { + this.addTenantIdInTargetStorage(tenantIdentifier); + } catch (DuplicateTenantException e) { + // ignore + } catch (StorageQueryException e) { + throw new IllegalStateException(e); + } + } + }); } catch (Exception e) { throw new DbInitException(e); } } + @Override + public synchronized void addTenantIdentifier(TenantIdentifier tenantIdentifier) { + this.tenantIdentifiers.add(tenantIdentifier); + } + @Override public T startTransaction(TransactionLogic logic) throws StorageTransactionLogicException, StorageQueryException { From 1ff1abc221db6458931a96c981c7d0ec852a868f Mon Sep 17 00:00:00 2001 From: Sattvik Chakravarthy Date: Mon, 15 Apr 2024 16:57:04 +0530 Subject: [PATCH 02/12] fix: exception --- src/main/java/io/supertokens/storage/postgresql/Start.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/supertokens/storage/postgresql/Start.java b/src/main/java/io/supertokens/storage/postgresql/Start.java index 21ddd8ef..a1f4a804 100644 --- a/src/main/java/io/supertokens/storage/postgresql/Start.java +++ b/src/main/java/io/supertokens/storage/postgresql/Start.java @@ -230,7 +230,7 @@ public void initStorage(boolean shouldWait) throws DbInitException { try { GeneralQueries.createTablesIfNotExists(this); } catch (Exception e2) { - throw new IllegalStateException(e2); + throw new RuntimeException(e2); } for (TenantIdentifier tenantIdentifier : this.tenantIdentifiers) { try { @@ -238,7 +238,7 @@ public void initStorage(boolean shouldWait) throws DbInitException { } catch (DuplicateTenantException e) { // ignore } catch (StorageQueryException e) { - throw new IllegalStateException(e); + throw new RuntimeException(e); } } }); From cf5b3b291c3006bd3b0d9915ddffc3b17f5ecaf8 Mon Sep 17 00:00:00 2001 From: Sattvik Chakravarthy Date: Tue, 16 Apr 2024 17:21:42 +0530 Subject: [PATCH 03/12] fix: PR comments --- .../storage/postgresql/ConnectionPool.java | 29 +- .../supertokens/storage/postgresql/Start.java | 6 +- .../queries/MultitenancyQueries.java | 6 + .../storage/postgresql/test/Utils.java | 3 + .../TestForNoCrashDuringStartup.java | 295 ++++++++++++++++++ 5 files changed, 329 insertions(+), 10 deletions(-) create mode 100644 src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java diff --git a/src/main/java/io/supertokens/storage/postgresql/ConnectionPool.java b/src/main/java/io/supertokens/storage/postgresql/ConnectionPool.java index 1f5d01a3..9fa9f392 100644 --- a/src/main/java/io/supertokens/storage/postgresql/ConnectionPool.java +++ b/src/main/java/io/supertokens/storage/postgresql/ConnectionPool.java @@ -19,7 +19,9 @@ import com.zaxxer.hikari.HikariConfig; import com.zaxxer.hikari.HikariDataSource; +import io.supertokens.pluginInterface.Storage; import io.supertokens.pluginInterface.exceptions.DbInitException; +import io.supertokens.pluginInterface.exceptions.StorageQueryException; import io.supertokens.storage.postgresql.config.Config; import io.supertokens.storage.postgresql.config.PostgreSQLConfig; import io.supertokens.storage.postgresql.output.Logging; @@ -35,14 +37,14 @@ public class ConnectionPool extends ResourceDistributor.SingletonResource { private static final String RESOURCE_KEY = "io.supertokens.storage.postgresql.ConnectionPool"; private HikariDataSource hikariDataSource; private final Start start; - private Runnable postConnectCallback; + private PostConnectCallback postConnectCallback; - private ConnectionPool(Start start, Runnable postConnectCallback) { + private ConnectionPool(Start start, PostConnectCallback postConnectCallback) { this.start = start; this.postConnectCallback = postConnectCallback; } - private synchronized void initialiseHikariDataSource() throws SQLException { + private synchronized void initialiseHikariDataSource() throws SQLException, StorageQueryException { if (this.hikariDataSource != null) { return; } @@ -98,10 +100,20 @@ private synchronized void initialiseHikariDataSource() throws SQLException { config.setPoolName(start.getUserPoolId() + "~" + start.getConnectionPoolId()); try { hikariDataSource = new HikariDataSource(config); - this.postConnectCallback.run(); } catch (Exception e) { throw new SQLException(e); } + + try { + this.postConnectCallback.apply(); + } catch (StorageQueryException e) { + // if an exception happens here, we want to set the hikariDataSource to null once again so that + // whenever the getConnection is called again, we want to re-attempt creation of tables and tenant + // entries for this storage + hikariDataSource.close(); + hikariDataSource = null; + throw e; + } } private static int getTimeToWaitToInit(Start start) { @@ -136,7 +148,7 @@ static boolean isAlreadyInitialised(Start start) { return getInstance(start) != null && getInstance(start).hikariDataSource != null; } - static void initPool(Start start, boolean shouldWait, Runnable postConnectCallback) throws DbInitException { + static void initPool(Start start, boolean shouldWait, PostConnectCallback postConnectCallback) throws DbInitException { if (isAlreadyInitialised(start)) { return; } @@ -192,7 +204,7 @@ static void initPool(Start start, boolean shouldWait, Runnable postConnectCallba } } - public static Connection getConnection(Start start) throws SQLException { + public static Connection getConnection(Start start) throws SQLException, StorageQueryException { if (getInstance(start) == null) { throw new IllegalStateException("Please call initPool before getConnection"); } @@ -219,4 +231,9 @@ static void close(Start start) { } } } + + @FunctionalInterface + public static interface PostConnectCallback { + void apply() throws StorageQueryException; + } } diff --git a/src/main/java/io/supertokens/storage/postgresql/Start.java b/src/main/java/io/supertokens/storage/postgresql/Start.java index a1f4a804..db5a732e 100644 --- a/src/main/java/io/supertokens/storage/postgresql/Start.java +++ b/src/main/java/io/supertokens/storage/postgresql/Start.java @@ -229,16 +229,14 @@ public void initStorage(boolean shouldWait) throws DbInitException { ConnectionPool.initPool(this, shouldWait, () -> { try { GeneralQueries.createTablesIfNotExists(this); - } catch (Exception e2) { - throw new RuntimeException(e2); + } catch (SQLException e) { + throw new StorageQueryException(e); } for (TenantIdentifier tenantIdentifier : this.tenantIdentifiers) { try { this.addTenantIdInTargetStorage(tenantIdentifier); } catch (DuplicateTenantException e) { // ignore - } catch (StorageQueryException e) { - throw new RuntimeException(e); } } }); diff --git a/src/main/java/io/supertokens/storage/postgresql/queries/MultitenancyQueries.java b/src/main/java/io/supertokens/storage/postgresql/queries/MultitenancyQueries.java index e4801d45..b2406003 100644 --- a/src/main/java/io/supertokens/storage/postgresql/queries/MultitenancyQueries.java +++ b/src/main/java/io/supertokens/storage/postgresql/queries/MultitenancyQueries.java @@ -37,6 +37,8 @@ import static io.supertokens.storage.postgresql.config.Config.getConfig; public class MultitenancyQueries { + public static boolean simulateErrorInAddingTenantIdInTargetStorage = false; + static String getQueryToCreateTenantConfigsTable(Start start) { String schema = Config.getConfig(start).getTableSchema(); String tenantConfigsTable = Config.getConfig(start).getTenantConfigsTable(); @@ -274,6 +276,10 @@ public static TenantConfig[] getAllTenants(Start start) throws StorageQueryExcep public static void addTenantIdInTargetStorage(Start start, TenantIdentifier tenantIdentifier) throws StorageTransactionLogicException, StorageQueryException { { + if (Start.isTesting && simulateErrorInAddingTenantIdInTargetStorage) { + throw new StorageTransactionLogicException(new SQLException("Simulated error in addTenantIdInTargetStorage")); + } + start.startTransaction(con -> { Connection sqlCon = (Connection) con.getConnection(); long currentTime = System.currentTimeMillis(); diff --git a/src/test/java/io/supertokens/storage/postgresql/test/Utils.java b/src/test/java/io/supertokens/storage/postgresql/test/Utils.java index 161d2e8c..f06a1299 100644 --- a/src/test/java/io/supertokens/storage/postgresql/test/Utils.java +++ b/src/test/java/io/supertokens/storage/postgresql/test/Utils.java @@ -20,6 +20,7 @@ import io.supertokens.Main; import io.supertokens.pluginInterface.PluginInterfaceTesting; import io.supertokens.storage.postgresql.Start; +import io.supertokens.storage.postgresql.queries.MultitenancyQueries; import io.supertokens.storageLayer.StorageLayer; import org.apache.tomcat.util.http.fileupload.FileUtils; import org.junit.rules.TestRule; @@ -78,6 +79,8 @@ public static void reset() { PluginInterfaceTesting.isTesting = true; Start.isTesting = true; Main.makeConsolePrintSilent = true; + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage = false; + String installDir = "../"; try { // if the default config is not the same as the current config, we must reset diff --git a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java new file mode 100644 index 00000000..00ed3413 --- /dev/null +++ b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java @@ -0,0 +1,295 @@ +/* + * Copyright (c) 2024, VRAI Labs and/or its affiliates. All rights reserved. + * + * This software is licensed under the Apache License, Version 2.0 (the + * "License") as published by the Apache Software Foundation. + * + * You may not use this file except in compliance with the License. You may + * obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ + +package io.supertokens.storage.postgresql.test.multitenancy; + +import com.google.gson.JsonObject; +import io.supertokens.Main; +import io.supertokens.ProcessState; +import io.supertokens.featureflag.EE_FEATURES; +import io.supertokens.featureflag.FeatureFlagTestContent; +import io.supertokens.featureflag.exceptions.FeatureNotEnabledException; +import io.supertokens.multitenancy.Multitenancy; +import io.supertokens.multitenancy.MultitenancyHelper; +import io.supertokens.multitenancy.exception.BadPermissionException; +import io.supertokens.multitenancy.exception.CannotModifyBaseConfigException; +import io.supertokens.pluginInterface.exceptions.InvalidConfigException; +import io.supertokens.pluginInterface.exceptions.StorageQueryException; +import io.supertokens.pluginInterface.multitenancy.*; +import io.supertokens.pluginInterface.multitenancy.exceptions.TenantOrAppNotFoundException; +import io.supertokens.storage.postgresql.Start; +import io.supertokens.storage.postgresql.queries.MultitenancyQueries; +import io.supertokens.storage.postgresql.test.TestingProcessManager; +import io.supertokens.storage.postgresql.test.Utils; +import io.supertokens.storage.postgresql.test.httpRequest.HttpRequestForTesting; +import io.supertokens.storage.postgresql.test.httpRequest.HttpResponseException; +import io.supertokens.storageLayer.StorageLayer; +import io.supertokens.thirdparty.InvalidProviderConfigException; +import io.supertokens.utils.SemVer; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; + +import static org.junit.Assert.*; +import static io.supertokens.storage.postgresql.QueryExecutorTemplate.update; + +public class TestForNoCrashDuringStartup { + TestingProcessManager.TestingProcess process; + + @AfterClass + public static void afterTesting() { + Utils.afterTesting(); + } + + @After + public void afterEach() throws InterruptedException { + process.kill(); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); + } + + @Before + public void beforeEach() throws InterruptedException, InvalidProviderConfigException, + StorageQueryException, FeatureNotEnabledException, TenantOrAppNotFoundException, IOException, + InvalidConfigException, CannotModifyBaseConfigException, BadPermissionException { + Utils.reset(); + + String[] args = {"../"}; + + this.process = TestingProcessManager.start(args); + FeatureFlagTestContent.getInstance(process.getProcess()) + .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{EE_FEATURES.MULTI_TENANCY}); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED)); + } + + @Test + public void testFailureInCUDCreation() throws Exception { + JsonObject coreConfig = new JsonObject(); + StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess()) + .modifyConfigToAddANewUserPoolForTesting(coreConfig, 1); + + TenantIdentifier tenantIdentifier = new TenantIdentifier("127.0.0.1", null, null); + + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage = true; + try { + Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig( + tenantIdentifier, + new EmailPasswordConfig(true), + new ThirdPartyConfig(true, null), + new PasswordlessConfig(true), + null, null, + coreConfig + ), false); + fail(); + } catch (StorageQueryException e) { + // ignore + assertTrue(e.getMessage().contains("Simulated error")); + } + + TenantConfig[] allTenants = MultitenancyHelper.getInstance(process.getProcess()).getAllTenants(); + assertEquals(2, allTenants.length); // should have the new CUD + + try { + tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); + fail(); + } catch (HttpResponseException e) { + // ignore + assertTrue(e.getMessage().contains("Internal Error")); // retried creating tenant entry + } + + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage = false; + + // this should succeed now + tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); + } + + @Test + public void testThatTenantComesToLifeOnceTheTargetDbIsUp() throws Exception { + Start start = ((Start) StorageLayer.getBaseStorage(process.getProcess())); + try { + update(start, "DROP DATABASE st5000;", pst -> {}); + } catch (Exception e) { + // ignore + } + + JsonObject coreConfig = new JsonObject(); + StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess()) + .modifyConfigToAddANewUserPoolForTesting(coreConfig, 5000); + + TenantIdentifier tenantIdentifier = new TenantIdentifier("127.0.0.1", null, null); + + try { + Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig( + tenantIdentifier, + new EmailPasswordConfig(true), + new ThirdPartyConfig(true, null), + new PasswordlessConfig(true), + null, null, + coreConfig + ), false); + fail(); + } catch (StorageQueryException e) { + // ignore + assertEquals("java.sql.SQLException: com.zaxxer.hikari.pool.HikariPool$PoolInitializationException: Failed to initialize pool: FATAL: database \"st5000\" does not exist", e.getMessage()); + } + + TenantConfig[] allTenants = MultitenancyHelper.getInstance(process.getProcess()).getAllTenants(); + assertEquals(2, allTenants.length); // should have the new CUD + + try { + tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); + fail(); + } catch (HttpResponseException e) { + // ignore + assertTrue(e.getMessage().contains("Internal Error")); // db is still down + } + + update(start, "CREATE DATABASE st5000;", pst -> {}); + + // this should succeed now + tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); + } + + @Test + public void testThatCoreDoesNotStartWithThereIsAnErrorDuringTenantCreation() throws Exception { + JsonObject coreConfig = new JsonObject(); + StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess()) + .modifyConfigToAddANewUserPoolForTesting(coreConfig, 1); + + TenantIdentifier tenantIdentifier = new TenantIdentifier("127.0.0.1", null, null); + + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage = true; + try { + Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig( + tenantIdentifier, + new EmailPasswordConfig(true), + new ThirdPartyConfig(true, null), + new PasswordlessConfig(true), + null, null, + coreConfig + ), false); + fail(); + } catch (StorageQueryException e) { + // ignore + assertTrue(e.getMessage().contains("Simulated error")); + } + + TenantConfig[] allTenants = MultitenancyHelper.getInstance(process.getProcess()).getAllTenants(); + assertEquals(2, allTenants.length); // should have the new CUD + + try { + tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); + fail(); + } catch (HttpResponseException e) { + // ignore + assertTrue(e.getMessage().contains("Internal Error")); // retried creating tenant entry + } + + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage = false; + + process.kill(false); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); + + String[] args = {"../"}; + + this.process = TestingProcessManager.start(args); + FeatureFlagTestContent.getInstance(process.getProcess()) + .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{EE_FEATURES.MULTI_TENANCY}); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED)); + + // this should succeed now + tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); + } + + @Test + public void testThatTenantComesToLifeOnceTheTargetDbIsUpAfterCoreRestart() throws Exception { + Start start = ((Start) StorageLayer.getBaseStorage(process.getProcess())); + try { + update(start, "DROP DATABASE st5000;", pst -> {}); + } catch (Exception e) { + // ignore + } + + JsonObject coreConfig = new JsonObject(); + StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess()) + .modifyConfigToAddANewUserPoolForTesting(coreConfig, 5000); + + TenantIdentifier tenantIdentifier = new TenantIdentifier("127.0.0.1", null, null); + + try { + Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig( + tenantIdentifier, + new EmailPasswordConfig(true), + new ThirdPartyConfig(true, null), + new PasswordlessConfig(true), + null, null, + coreConfig + ), false); + fail(); + } catch (StorageQueryException e) { + // ignore + assertEquals("java.sql.SQLException: com.zaxxer.hikari.pool.HikariPool$PoolInitializationException: Failed to initialize pool: FATAL: database \"st5000\" does not exist", e.getMessage()); + } + + TenantConfig[] allTenants = MultitenancyHelper.getInstance(process.getProcess()).getAllTenants(); + assertEquals(2, allTenants.length); // should have the new CUD + + try { + tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); + fail(); + } catch (HttpResponseException e) { + // ignore + assertTrue(e.getMessage().contains("Internal Error")); // db is still down + } + + process.kill(false); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); + + String[] args = {"../"}; + this.process = TestingProcessManager.start(args); + FeatureFlagTestContent.getInstance(process.getProcess()) + .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{EE_FEATURES.MULTI_TENANCY}); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED)); + + // the process should start successfully even though the db is down + + start = ((Start) StorageLayer.getBaseStorage(process.getProcess())); + update(start, "CREATE DATABASE st5000;", pst -> {}); + + // this should succeed now + tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); + } + + public static JsonObject tpSignInUpAndGetResponse(TenantIdentifier tenantIdentifier, String thirdPartyId, String thirdPartyUserId, String email, Main main, SemVer version) + throws HttpResponseException, IOException { + JsonObject emailObject = new JsonObject(); + emailObject.addProperty("id", email); + emailObject.addProperty("isVerified", false); + + JsonObject signUpRequestBody = new JsonObject(); + signUpRequestBody.addProperty("thirdPartyId", thirdPartyId); + signUpRequestBody.addProperty("thirdPartyUserId", thirdPartyUserId); + signUpRequestBody.add("email", emailObject); + + JsonObject response = HttpRequestForTesting.sendJsonPOSTRequest(main, "", + HttpRequestForTesting.getMultitenantUrl(tenantIdentifier, "/recipe/signinup"), signUpRequestBody, + 1000, 1000, null, + version.get(), "thirdparty"); + return response; + } +} From 2955daaef6796a3e32a4ad03efa8919785004b86 Mon Sep 17 00:00:00 2001 From: Sattvik Chakravarthy Date: Tue, 16 Apr 2024 17:27:15 +0530 Subject: [PATCH 04/12] fix: update test --- .../test/multitenancy/TestForNoCrashDuringStartup.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java index 00ed3413..ee0d47cb 100644 --- a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java +++ b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java @@ -192,14 +192,6 @@ public void testThatCoreDoesNotStartWithThereIsAnErrorDuringTenantCreation() thr TenantConfig[] allTenants = MultitenancyHelper.getInstance(process.getProcess()).getAllTenants(); assertEquals(2, allTenants.length); // should have the new CUD - try { - tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); - fail(); - } catch (HttpResponseException e) { - // ignore - assertTrue(e.getMessage().contains("Internal Error")); // retried creating tenant entry - } - MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage = false; process.kill(false); From 4b41024697b67dff45be56d07037f611068c7488 Mon Sep 17 00:00:00 2001 From: Sattvik Chakravarthy Date: Tue, 16 Apr 2024 17:41:48 +0530 Subject: [PATCH 05/12] fix: update test --- .../test/multitenancy/TestForNoCrashDuringStartup.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java index ee0d47cb..b7c3bb9d 100644 --- a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java +++ b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java @@ -192,8 +192,6 @@ public void testThatCoreDoesNotStartWithThereIsAnErrorDuringTenantCreation() thr TenantConfig[] allTenants = MultitenancyHelper.getInstance(process.getProcess()).getAllTenants(); assertEquals(2, allTenants.length); // should have the new CUD - MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage = false; - process.kill(false); assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); @@ -204,6 +202,11 @@ public void testThatCoreDoesNotStartWithThereIsAnErrorDuringTenantCreation() thr .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{EE_FEATURES.MULTI_TENANCY}); assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED)); + allTenants = MultitenancyHelper.getInstance(process.getProcess()).getAllTenants(); + assertEquals(2, allTenants.length); // should have the new CUD + + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage = false; + // this should succeed now tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); } From 93030dca16c26182f090b01a504d95342286670b Mon Sep 17 00:00:00 2001 From: Sattvik Chakravarthy Date: Tue, 16 Apr 2024 17:59:06 +0530 Subject: [PATCH 06/12] fix: update test --- .../TestForNoCrashDuringStartup.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java index b7c3bb9d..34ed0da1 100644 --- a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java +++ b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java @@ -19,6 +19,7 @@ import com.google.gson.JsonObject; import io.supertokens.Main; import io.supertokens.ProcessState; +import io.supertokens.config.Config; import io.supertokens.featureflag.EE_FEATURES; import io.supertokens.featureflag.FeatureFlagTestContent; import io.supertokens.featureflag.exceptions.FeatureNotEnabledException; @@ -44,7 +45,10 @@ import org.junit.Before; import org.junit.Test; +import java.io.File; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Scanner; import static org.junit.Assert.*; import static io.supertokens.storage.postgresql.QueryExecutorTemplate.update; @@ -205,6 +209,23 @@ public void testThatCoreDoesNotStartWithThereIsAnErrorDuringTenantCreation() thr allTenants = MultitenancyHelper.getInstance(process.getProcess()).getAllTenants(); assertEquals(2, allTenants.length); // should have the new CUD + File errorLog = new File(Config.getConfig(process.getProcess()).getErrorLogPath(process.getProcess())); + + boolean foundSimulatedError = false; + + try (Scanner scanner = new Scanner(errorLog, StandardCharsets.UTF_8)) { + while (scanner.hasNextLine()) { + String line = scanner.nextLine(); + System.out.println(line); + if (line.contains("Simulated error")) { + foundSimulatedError = true; + break; + } + } + } + + assertTrue(foundSimulatedError); + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage = false; // this should succeed now From 9abec89f4b8f97d7c878952605fdf1f531b98c1c Mon Sep 17 00:00:00 2001 From: Sattvik Chakravarthy Date: Wed, 17 Apr 2024 12:59:27 +0530 Subject: [PATCH 07/12] fix: version --- CHANGELOG.md | 4 ++++ build.gradle | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cdc2ad60..a99afbf0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +## [7.0.1] - 2024-04-17 + +- Fixes issues with partial failures during tenant creation + ## [7.0.0] - 2024-03-13 - Replace `TotpNotEnabledError` with `UnknownUserIdTotpError`. diff --git a/build.gradle b/build.gradle index c1281047..cbd69075 100644 --- a/build.gradle +++ b/build.gradle @@ -2,7 +2,7 @@ plugins { id 'java-library' } -version = "7.0.0" +version = "7.0.1" repositories { mavenCentral() From 10201eaae0d72142843317847ec73b7e28bffe78 Mon Sep 17 00:00:00 2001 From: Sattvik Chakravarthy Date: Wed, 17 Apr 2024 13:12:35 +0530 Subject: [PATCH 08/12] fix: rename --- .../storage/postgresql/queries/MultitenancyQueries.java | 4 ++-- .../io/supertokens/storage/postgresql/test/Utils.java | 2 +- .../test/multitenancy/TestForNoCrashDuringStartup.java | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/io/supertokens/storage/postgresql/queries/MultitenancyQueries.java b/src/main/java/io/supertokens/storage/postgresql/queries/MultitenancyQueries.java index b2406003..179c12a0 100644 --- a/src/main/java/io/supertokens/storage/postgresql/queries/MultitenancyQueries.java +++ b/src/main/java/io/supertokens/storage/postgresql/queries/MultitenancyQueries.java @@ -37,7 +37,7 @@ import static io.supertokens.storage.postgresql.config.Config.getConfig; public class MultitenancyQueries { - public static boolean simulateErrorInAddingTenantIdInTargetStorage = false; + public static boolean simulateErrorInAddingTenantIdInTargetStorage_forTesting = false; static String getQueryToCreateTenantConfigsTable(Start start) { String schema = Config.getConfig(start).getTableSchema(); @@ -276,7 +276,7 @@ public static TenantConfig[] getAllTenants(Start start) throws StorageQueryExcep public static void addTenantIdInTargetStorage(Start start, TenantIdentifier tenantIdentifier) throws StorageTransactionLogicException, StorageQueryException { { - if (Start.isTesting && simulateErrorInAddingTenantIdInTargetStorage) { + if (Start.isTesting && simulateErrorInAddingTenantIdInTargetStorage_forTesting) { throw new StorageTransactionLogicException(new SQLException("Simulated error in addTenantIdInTargetStorage")); } diff --git a/src/test/java/io/supertokens/storage/postgresql/test/Utils.java b/src/test/java/io/supertokens/storage/postgresql/test/Utils.java index f06a1299..b82c4bfd 100644 --- a/src/test/java/io/supertokens/storage/postgresql/test/Utils.java +++ b/src/test/java/io/supertokens/storage/postgresql/test/Utils.java @@ -79,7 +79,7 @@ public static void reset() { PluginInterfaceTesting.isTesting = true; Start.isTesting = true; Main.makeConsolePrintSilent = true; - MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage = false; + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage_forTesting = false; String installDir = "../"; try { diff --git a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java index 34ed0da1..293cf6d2 100644 --- a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java +++ b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java @@ -89,7 +89,7 @@ public void testFailureInCUDCreation() throws Exception { TenantIdentifier tenantIdentifier = new TenantIdentifier("127.0.0.1", null, null); - MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage = true; + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage_forTesting = true; try { Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig( tenantIdentifier, @@ -116,7 +116,7 @@ public void testFailureInCUDCreation() throws Exception { assertTrue(e.getMessage().contains("Internal Error")); // retried creating tenant entry } - MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage = false; + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage_forTesting = false; // this should succeed now tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); @@ -177,7 +177,7 @@ public void testThatCoreDoesNotStartWithThereIsAnErrorDuringTenantCreation() thr TenantIdentifier tenantIdentifier = new TenantIdentifier("127.0.0.1", null, null); - MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage = true; + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage_forTesting = true; try { Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig( tenantIdentifier, @@ -226,7 +226,7 @@ public void testThatCoreDoesNotStartWithThereIsAnErrorDuringTenantCreation() thr assertTrue(foundSimulatedError); - MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage = false; + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage_forTesting = false; // this should succeed now tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); From 2b91b356d1ae9d280795d15c1ce3616a8f1351f2 Mon Sep 17 00:00:00 2001 From: Sattvik Chakravarthy Date: Thu, 18 Apr 2024 15:35:05 +0530 Subject: [PATCH 09/12] fix: pr comments --- pluginInterfaceSupported.json | 2 +- .../storage/postgresql/ConnectionPool.java | 7 +- .../supertokens/storage/postgresql/Start.java | 22 +- .../postgresql/queries/GeneralQueries.java | 249 +++++++++--------- .../queries/MultitenancyQueries.java | 68 +++-- .../TestForNoCrashDuringStartup.java | 58 ++++ 6 files changed, 251 insertions(+), 155 deletions(-) diff --git a/pluginInterfaceSupported.json b/pluginInterfaceSupported.json index f9d5be77..476e2b85 100644 --- a/pluginInterfaceSupported.json +++ b/pluginInterfaceSupported.json @@ -1,6 +1,6 @@ { "_comment": "contains a list of plugin interfaces branch names that this core supports", "versions": [ - "6.0" + "6.1" ] } \ No newline at end of file diff --git a/src/main/java/io/supertokens/storage/postgresql/ConnectionPool.java b/src/main/java/io/supertokens/storage/postgresql/ConnectionPool.java index 9fa9f392..534e706a 100644 --- a/src/main/java/io/supertokens/storage/postgresql/ConnectionPool.java +++ b/src/main/java/io/supertokens/storage/postgresql/ConnectionPool.java @@ -19,7 +19,6 @@ import com.zaxxer.hikari.HikariConfig; import com.zaxxer.hikari.HikariDataSource; -import io.supertokens.pluginInterface.Storage; import io.supertokens.pluginInterface.exceptions.DbInitException; import io.supertokens.pluginInterface.exceptions.StorageQueryException; import io.supertokens.storage.postgresql.config.Config; @@ -105,7 +104,9 @@ private synchronized void initialiseHikariDataSource() throws SQLException, Stor } try { - this.postConnectCallback.apply(); + try (Connection con = hikariDataSource.getConnection()) { + this.postConnectCallback.apply(con); + } } catch (StorageQueryException e) { // if an exception happens here, we want to set the hikariDataSource to null once again so that // whenever the getConnection is called again, we want to re-attempt creation of tables and tenant @@ -234,6 +235,6 @@ static void close(Start start) { @FunctionalInterface public static interface PostConnectCallback { - void apply() throws StorageQueryException; + void apply(Connection connection) throws StorageQueryException; } } diff --git a/src/main/java/io/supertokens/storage/postgresql/Start.java b/src/main/java/io/supertokens/storage/postgresql/Start.java index db5a732e..f76ee735 100644 --- a/src/main/java/io/supertokens/storage/postgresql/Start.java +++ b/src/main/java/io/supertokens/storage/postgresql/Start.java @@ -226,15 +226,15 @@ public void initStorage(boolean shouldWait) throws DbInitException { mainThread = Thread.currentThread(); } try { - ConnectionPool.initPool(this, shouldWait, () -> { + ConnectionPool.initPool(this, shouldWait, (con) -> { try { - GeneralQueries.createTablesIfNotExists(this); + GeneralQueries.createTablesIfNotExists(this, con); } catch (SQLException e) { throw new StorageQueryException(e); } for (TenantIdentifier tenantIdentifier : this.tenantIdentifiers) { try { - this.addTenantIdInTargetStorage(tenantIdentifier); + this.addTenantIdInTargetStorage_Transaction(con, tenantIdentifier); } catch (DuplicateTenantException e) { // ignore } @@ -2298,6 +2298,22 @@ public void addTenantIdInTargetStorage(TenantIdentifier tenantIdentifier) } } + public void addTenantIdInTargetStorage_Transaction(Connection con, TenantIdentifier tenantIdentifier) + throws DuplicateTenantException, StorageQueryException { + try { + MultitenancyQueries.addTenantIdInTargetStorage_Transaction(this, con, tenantIdentifier); + } catch (SQLException e) { + if (e instanceof PSQLException) { + PostgreSQLConfig config = Config.getConfig(this); + if (isPrimaryKeyError(((PSQLException) e).getServerErrorMessage(), + config.getTenantsTable())) { + throw new DuplicateTenantException(); + } + } + throw new StorageQueryException(e); + } + } + @Override public void overwriteTenantConfig(TenantConfig tenantConfig) throws TenantOrAppNotFoundException, StorageQueryException, DuplicateThirdPartyIdException, diff --git a/src/main/java/io/supertokens/storage/postgresql/queries/GeneralQueries.java b/src/main/java/io/supertokens/storage/postgresql/queries/GeneralQueries.java index 8bc2d561..94b54514 100644 --- a/src/main/java/io/supertokens/storage/postgresql/queries/GeneralQueries.java +++ b/src/main/java/io/supertokens/storage/postgresql/queries/GeneralQueries.java @@ -56,13 +56,16 @@ public class GeneralQueries { - private static boolean doesTableExists(Start start, String tableName) { + private static boolean doesTableExists(Start start, Connection connection, String tableName) throws SQLException, StorageQueryException { try { String QUERY = "SELECT 1 FROM " + tableName + " LIMIT 1"; - execute(start, QUERY, NO_OP_SETTER, result -> null); + execute(connection, QUERY, NO_OP_SETTER, result -> null); return true; } catch (SQLException | StorageQueryException e) { - return false; + if (e.getMessage().contains("relation") && e.getMessage().contains(tableName) && e.getMessage().contains("does not exist")) { + return false; + } + throw e; } } @@ -236,213 +239,213 @@ static String getQueryToCreatePrimaryUserIdIndexForAppIdToUserIdTable(Start star + Config.getConfig(start).getAppIdToUserIdTable() + "(primary_or_recipe_user_id, app_id);"; } - public static void createTablesIfNotExists(Start start) throws SQLException, StorageQueryException { + public static void createTablesIfNotExists(Start start, Connection con) throws SQLException, StorageQueryException { int numberOfRetries = 0; boolean retry = true; while (retry) { retry = false; try { - if (!doesTableExists(start, Config.getConfig(start).getAppsTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getAppsTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, getQueryToCreateAppsTable(start), NO_OP_SETTER); + update(con, getQueryToCreateAppsTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getTenantsTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getTenantsTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, getQueryToCreateTenantsTable(start), NO_OP_SETTER); + update(con, getQueryToCreateTenantsTable(start), NO_OP_SETTER); // index - update(start, getQueryToCreateAppIdIndexForTenantsTable(start), NO_OP_SETTER); + update(con, getQueryToCreateAppIdIndexForTenantsTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getKeyValueTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getKeyValueTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, getQueryToCreateKeyValueTable(start), NO_OP_SETTER); + update(con, getQueryToCreateKeyValueTable(start), NO_OP_SETTER); // index - update(start, getQueryToCreateTenantIdIndexForKeyValueTable(start), NO_OP_SETTER); + update(con, getQueryToCreateTenantIdIndexForKeyValueTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getAppIdToUserIdTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getAppIdToUserIdTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, getQueryToCreateAppIdToUserIdTable(start), NO_OP_SETTER); + update(con, getQueryToCreateAppIdToUserIdTable(start), NO_OP_SETTER); // index - update(start, getQueryToCreateAppIdIndexForAppIdToUserIdTable(start), NO_OP_SETTER); - update(start, getQueryToCreatePrimaryUserIdIndexForAppIdToUserIdTable(start), NO_OP_SETTER); + update(con, getQueryToCreateAppIdIndexForAppIdToUserIdTable(start), NO_OP_SETTER); + update(con, getQueryToCreatePrimaryUserIdIndexForAppIdToUserIdTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getUsersTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getUsersTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, getQueryToCreateUsersTable(start), NO_OP_SETTER); + update(con, getQueryToCreateUsersTable(start), NO_OP_SETTER); // index - update(start, getQueryToCreateUserPaginationIndex1(start), NO_OP_SETTER); - update(start, getQueryToCreateUserPaginationIndex2(start), NO_OP_SETTER); - update(start, getQueryToCreateUserPaginationIndex3(start), NO_OP_SETTER); - update(start, getQueryToCreateUserPaginationIndex4(start), NO_OP_SETTER); - update(start, getQueryToCreatePrimaryUserId(start), NO_OP_SETTER); - update(start, getQueryToCreateRecipeIdIndex(start), NO_OP_SETTER); + update(con, getQueryToCreateUserPaginationIndex1(start), NO_OP_SETTER); + update(con, getQueryToCreateUserPaginationIndex2(start), NO_OP_SETTER); + update(con, getQueryToCreateUserPaginationIndex3(start), NO_OP_SETTER); + update(con, getQueryToCreateUserPaginationIndex4(start), NO_OP_SETTER); + update(con, getQueryToCreatePrimaryUserId(start), NO_OP_SETTER); + update(con, getQueryToCreateRecipeIdIndex(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getUserLastActiveTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getUserLastActiveTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, ActiveUsersQueries.getQueryToCreateUserLastActiveTable(start), NO_OP_SETTER); + update(con, ActiveUsersQueries.getQueryToCreateUserLastActiveTable(start), NO_OP_SETTER); // Index - update(start, ActiveUsersQueries.getQueryToCreateAppIdIndexForUserLastActiveTable(start), + update(con, ActiveUsersQueries.getQueryToCreateAppIdIndexForUserLastActiveTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getAccessTokenSigningKeysTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getAccessTokenSigningKeysTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, getQueryToCreateAccessTokenSigningKeysTable(start), NO_OP_SETTER); + update(con, getQueryToCreateAccessTokenSigningKeysTable(start), NO_OP_SETTER); // index - update(start, getQueryToCreateAppIdIndexForAccessTokenSigningKeysTable(start), NO_OP_SETTER); + update(con, getQueryToCreateAppIdIndexForAccessTokenSigningKeysTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getSessionInfoTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getSessionInfoTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, getQueryToCreateSessionInfoTable(start), NO_OP_SETTER); + update(con, getQueryToCreateSessionInfoTable(start), NO_OP_SETTER); // index - update(start, getQueryToCreateSessionExpiryIndex(start), NO_OP_SETTER); - update(start, getQueryToCreateTenantIdIndexForSessionInfoTable(start), NO_OP_SETTER); + update(con, getQueryToCreateSessionExpiryIndex(start), NO_OP_SETTER); + update(con, getQueryToCreateTenantIdIndexForSessionInfoTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getTenantConfigsTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getTenantConfigsTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, MultitenancyQueries.getQueryToCreateTenantConfigsTable(start), NO_OP_SETTER); + update(con, MultitenancyQueries.getQueryToCreateTenantConfigsTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getTenantThirdPartyProvidersTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getTenantThirdPartyProvidersTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, MultitenancyQueries.getQueryToCreateTenantThirdPartyProvidersTable(start), + update(con, MultitenancyQueries.getQueryToCreateTenantThirdPartyProvidersTable(start), NO_OP_SETTER); // index - update(start, + update(con, MultitenancyQueries.getQueryToCreateTenantIdIndexForTenantThirdPartyProvidersTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getTenantFirstFactorsTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getTenantFirstFactorsTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, MultitenancyQueries.getQueryToCreateFirstFactorsTable(start), NO_OP_SETTER); + update(con, MultitenancyQueries.getQueryToCreateFirstFactorsTable(start), NO_OP_SETTER); // index - update(start, MultitenancyQueries.getQueryToCreateTenantIdIndexForFirstFactorsTable(start), + update(con, MultitenancyQueries.getQueryToCreateTenantIdIndexForFirstFactorsTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getTenantRequiredSecondaryFactorsTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getTenantRequiredSecondaryFactorsTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, MultitenancyQueries.getQueryToCreateRequiredSecondaryFactorsTable(start), NO_OP_SETTER); + update(con, MultitenancyQueries.getQueryToCreateRequiredSecondaryFactorsTable(start), NO_OP_SETTER); // index - update(start, + update(con, MultitenancyQueries.getQueryToCreateTenantIdIndexForRequiredSecondaryFactorsTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getTenantThirdPartyProviderClientsTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getTenantThirdPartyProviderClientsTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, MultitenancyQueries.getQueryToCreateTenantThirdPartyProviderClientsTable(start), + update(con, MultitenancyQueries.getQueryToCreateTenantThirdPartyProviderClientsTable(start), NO_OP_SETTER); // index - update(start, + update(con, MultitenancyQueries.getQueryToCreateThirdPartyIdIndexForTenantThirdPartyProviderClientsTable( start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getEmailPasswordUsersTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getEmailPasswordUsersTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, EmailPasswordQueries.getQueryToCreateUsersTable(start), NO_OP_SETTER); + update(con, EmailPasswordQueries.getQueryToCreateUsersTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getEmailPasswordUserToTenantTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getEmailPasswordUserToTenantTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, EmailPasswordQueries.getQueryToCreateEmailPasswordUserToTenantTable(start), + update(con, EmailPasswordQueries.getQueryToCreateEmailPasswordUserToTenantTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getPasswordResetTokensTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getPasswordResetTokensTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, getQueryToCreatePasswordResetTokensTable(start), NO_OP_SETTER); + update(con, getQueryToCreatePasswordResetTokensTable(start), NO_OP_SETTER); // index - update(start, getQueryToCreatePasswordResetTokenExpiryIndex(start), NO_OP_SETTER); - update(start, getQueryToCreateUserIdIndexForPasswordResetTokensTable(start), NO_OP_SETTER); + update(con, getQueryToCreatePasswordResetTokenExpiryIndex(start), NO_OP_SETTER); + update(con, getQueryToCreateUserIdIndexForPasswordResetTokensTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getEmailVerificationTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getEmailVerificationTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, getQueryToCreateEmailVerificationTable(start), NO_OP_SETTER); + update(con, getQueryToCreateEmailVerificationTable(start), NO_OP_SETTER); // index - update(start, getQueryToCreateAppIdIndexForEmailVerificationTable(start), NO_OP_SETTER); + update(con, getQueryToCreateAppIdIndexForEmailVerificationTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getEmailVerificationTokensTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getEmailVerificationTokensTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, getQueryToCreateEmailVerificationTokensTable(start), NO_OP_SETTER); + update(con, getQueryToCreateEmailVerificationTokensTable(start), NO_OP_SETTER); // index - update(start, getQueryToCreateEmailVerificationTokenExpiryIndex(start), NO_OP_SETTER); - update(start, getQueryToCreateTenantIdIndexForEmailVerificationTokensTable(start), NO_OP_SETTER); + update(con, getQueryToCreateEmailVerificationTokenExpiryIndex(start), NO_OP_SETTER); + update(con, getQueryToCreateTenantIdIndexForEmailVerificationTokensTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getThirdPartyUsersTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getThirdPartyUsersTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, ThirdPartyQueries.getQueryToCreateUsersTable(start), NO_OP_SETTER); + update(con, ThirdPartyQueries.getQueryToCreateUsersTable(start), NO_OP_SETTER); // index - update(start, ThirdPartyQueries.getQueryToThirdPartyUserEmailIndex(start), NO_OP_SETTER); - update(start, ThirdPartyQueries.getQueryToThirdPartyUserIdIndex(start), NO_OP_SETTER); + update(con, ThirdPartyQueries.getQueryToThirdPartyUserEmailIndex(start), NO_OP_SETTER); + update(con, ThirdPartyQueries.getQueryToThirdPartyUserIdIndex(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getThirdPartyUserToTenantTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getThirdPartyUserToTenantTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, ThirdPartyQueries.getQueryToCreateThirdPartyUserToTenantTable(start), NO_OP_SETTER); + update(con, ThirdPartyQueries.getQueryToCreateThirdPartyUserToTenantTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getJWTSigningKeysTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getJWTSigningKeysTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, getQueryToCreateJWTSigningTable(start), NO_OP_SETTER); + update(con, getQueryToCreateJWTSigningTable(start), NO_OP_SETTER); // index - update(start, getQueryToCreateAppIdIndexForJWTSigningTable(start), NO_OP_SETTER); + update(con, getQueryToCreateAppIdIndexForJWTSigningTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getPasswordlessUsersTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getPasswordlessUsersTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, PasswordlessQueries.getQueryToCreateUsersTable(start), NO_OP_SETTER); + update(con, PasswordlessQueries.getQueryToCreateUsersTable(start), NO_OP_SETTER); // index - update(start, getQueryToCreateUserIdIndexForUsersTable(start), NO_OP_SETTER); - update(start, getQueryToCreateTenantIdIndexForUsersTable(start), NO_OP_SETTER); + update(con, getQueryToCreateUserIdIndexForUsersTable(start), NO_OP_SETTER); + update(con, getQueryToCreateTenantIdIndexForUsersTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getPasswordlessUserToTenantTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getPasswordlessUserToTenantTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, PasswordlessQueries.getQueryToCreatePasswordlessUserToTenantTable(start), + update(con, PasswordlessQueries.getQueryToCreatePasswordlessUserToTenantTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getPasswordlessDevicesTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getPasswordlessDevicesTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, getQueryToCreateDevicesTable(start), NO_OP_SETTER); + update(con, getQueryToCreateDevicesTable(start), NO_OP_SETTER); // index - update(start, getQueryToCreateDeviceEmailIndex(start), NO_OP_SETTER); - update(start, getQueryToCreateDevicePhoneNumberIndex(start), NO_OP_SETTER); - update(start, getQueryToCreateTenantIdIndexForDevicesTable(start), NO_OP_SETTER); + update(con, getQueryToCreateDeviceEmailIndex(start), NO_OP_SETTER); + update(con, getQueryToCreateDevicePhoneNumberIndex(start), NO_OP_SETTER); + update(con, getQueryToCreateTenantIdIndexForDevicesTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getPasswordlessCodesTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getPasswordlessCodesTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, getQueryToCreateCodesTable(start), NO_OP_SETTER); + update(con, getQueryToCreateCodesTable(start), NO_OP_SETTER); // index - update(start, getQueryToCreateCodeCreatedAtIndex(start), NO_OP_SETTER); + update(con, getQueryToCreateCodeCreatedAtIndex(start), NO_OP_SETTER); } // This PostgreSQL specific, because it's created automatically in MySQL and it @@ -450,95 +453,95 @@ public static void createTablesIfNotExists(Start start) throws SQLException, Sto // index if not exists" // We missed creating this earlier for the codes table, so it may be missing // even if the table exists - update(start, getQueryToCreateCodeDeviceIdHashIndex(start), NO_OP_SETTER); + update(con, getQueryToCreateCodeDeviceIdHashIndex(start), NO_OP_SETTER); - if (!doesTableExists(start, Config.getConfig(start).getUserMetadataTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getUserMetadataTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, getQueryToCreateUserMetadataTable(start), NO_OP_SETTER); + update(con, getQueryToCreateUserMetadataTable(start), NO_OP_SETTER); // Index - update(start, getQueryToCreateAppIdIndexForUserMetadataTable(start), NO_OP_SETTER); + update(con, getQueryToCreateAppIdIndexForUserMetadataTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getRolesTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getRolesTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, UserRolesQueries.getQueryToCreateRolesTable(start), NO_OP_SETTER); + update(con, UserRolesQueries.getQueryToCreateRolesTable(start), NO_OP_SETTER); // Index - update(start, UserRolesQueries.getQueryToCreateAppIdIndexForRolesTable(start), NO_OP_SETTER); + update(con, UserRolesQueries.getQueryToCreateAppIdIndexForRolesTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getUserRolesPermissionsTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getUserRolesPermissionsTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, UserRolesQueries.getQueryToCreateRolePermissionsTable(start), NO_OP_SETTER); + update(con, UserRolesQueries.getQueryToCreateRolePermissionsTable(start), NO_OP_SETTER); // index - update(start, UserRolesQueries.getQueryToCreateRolePermissionsPermissionIndex(start), NO_OP_SETTER); - update(start, UserRolesQueries.getQueryToCreateRoleIndexForRolePermissionsTable(start), + update(con, UserRolesQueries.getQueryToCreateRolePermissionsPermissionIndex(start), NO_OP_SETTER); + update(con, UserRolesQueries.getQueryToCreateRoleIndexForRolePermissionsTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getUserRolesTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getUserRolesTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, UserRolesQueries.getQueryToCreateUserRolesTable(start), NO_OP_SETTER); + update(con, UserRolesQueries.getQueryToCreateUserRolesTable(start), NO_OP_SETTER); // index - update(start, UserRolesQueries.getQueryToCreateUserRolesRoleIndex(start), NO_OP_SETTER); - update(start, UserRolesQueries.getQueryToCreateTenantIdIndexForUserRolesTable(start), NO_OP_SETTER); - update(start, UserRolesQueries.getQueryToCreateRoleIndexForUserRolesTable(start), NO_OP_SETTER); + update(con, UserRolesQueries.getQueryToCreateUserRolesRoleIndex(start), NO_OP_SETTER); + update(con, UserRolesQueries.getQueryToCreateTenantIdIndexForUserRolesTable(start), NO_OP_SETTER); + update(con, UserRolesQueries.getQueryToCreateRoleIndexForUserRolesTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getUserIdMappingTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getUserIdMappingTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, UserIdMappingQueries.getQueryToCreateUserIdMappingTable(start), NO_OP_SETTER); + update(con, UserIdMappingQueries.getQueryToCreateUserIdMappingTable(start), NO_OP_SETTER); // index - update(start, + update(con, UserIdMappingQueries.getQueryToCreateSupertokensUserIdIndexForUserIdMappingTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getDashboardUsersTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getDashboardUsersTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, DashboardQueries.getQueryToCreateDashboardUsersTable(start), NO_OP_SETTER); + update(con, DashboardQueries.getQueryToCreateDashboardUsersTable(start), NO_OP_SETTER); // Index - update(start, DashboardQueries.getQueryToCreateAppIdIndexForDashboardUsersTable(start), + update(con, DashboardQueries.getQueryToCreateAppIdIndexForDashboardUsersTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getDashboardSessionsTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getDashboardSessionsTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, DashboardQueries.getQueryToCreateDashboardUserSessionsTable(start), NO_OP_SETTER); + update(con, DashboardQueries.getQueryToCreateDashboardUserSessionsTable(start), NO_OP_SETTER); // index - update(start, DashboardQueries.getQueryToCreateDashboardUserSessionsExpiryIndex(start), + update(con, DashboardQueries.getQueryToCreateDashboardUserSessionsExpiryIndex(start), NO_OP_SETTER); - update(start, DashboardQueries.getQueryToCreateUserIdIndexForDashboardUserSessionsTable(start), + update(con, DashboardQueries.getQueryToCreateUserIdIndexForDashboardUserSessionsTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getTotpUsersTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getTotpUsersTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, TOTPQueries.getQueryToCreateUsersTable(start), NO_OP_SETTER); + update(con, TOTPQueries.getQueryToCreateUsersTable(start), NO_OP_SETTER); // index - update(start, TOTPQueries.getQueryToCreateAppIdIndexForUsersTable(start), NO_OP_SETTER); + update(con, TOTPQueries.getQueryToCreateAppIdIndexForUsersTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getTotpUserDevicesTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getTotpUserDevicesTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, TOTPQueries.getQueryToCreateUserDevicesTable(start), NO_OP_SETTER); + update(con, TOTPQueries.getQueryToCreateUserDevicesTable(start), NO_OP_SETTER); // index - update(start, TOTPQueries.getQueryToCreateUserIdIndexForUserDevicesTable(start), NO_OP_SETTER); + update(con, TOTPQueries.getQueryToCreateUserIdIndexForUserDevicesTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getTotpUsedCodesTable())) { + if (!doesTableExists(start, con, Config.getConfig(start).getTotpUsedCodesTable())) { getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, TOTPQueries.getQueryToCreateUsedCodesTable(start), NO_OP_SETTER); + update(con, TOTPQueries.getQueryToCreateUsedCodesTable(start), NO_OP_SETTER); // index: - update(start, TOTPQueries.getQueryToCreateUsedCodesExpiryTimeIndex(start), NO_OP_SETTER); - update(start, TOTPQueries.getQueryToCreateUserIdIndexForUsedCodesTable(start), NO_OP_SETTER); - update(start, TOTPQueries.getQueryToCreateTenantIdIndexForUsedCodesTable(start), NO_OP_SETTER); + update(con, TOTPQueries.getQueryToCreateUsedCodesExpiryTimeIndex(start), NO_OP_SETTER); + update(con, TOTPQueries.getQueryToCreateUserIdIndexForUsedCodesTable(start), NO_OP_SETTER); + update(con, TOTPQueries.getQueryToCreateTenantIdIndexForUsedCodesTable(start), NO_OP_SETTER); } } catch (Exception e) { diff --git a/src/main/java/io/supertokens/storage/postgresql/queries/MultitenancyQueries.java b/src/main/java/io/supertokens/storage/postgresql/queries/MultitenancyQueries.java index 179c12a0..0d9bf826 100644 --- a/src/main/java/io/supertokens/storage/postgresql/queries/MultitenancyQueries.java +++ b/src/main/java/io/supertokens/storage/postgresql/queries/MultitenancyQueries.java @@ -276,34 +276,10 @@ public static TenantConfig[] getAllTenants(Start start) throws StorageQueryExcep public static void addTenantIdInTargetStorage(Start start, TenantIdentifier tenantIdentifier) throws StorageTransactionLogicException, StorageQueryException { { - if (Start.isTesting && simulateErrorInAddingTenantIdInTargetStorage_forTesting) { - throw new StorageTransactionLogicException(new SQLException("Simulated error in addTenantIdInTargetStorage")); - } - start.startTransaction(con -> { Connection sqlCon = (Connection) con.getConnection(); - long currentTime = System.currentTimeMillis(); try { - { - String QUERY = "INSERT INTO " + getConfig(start).getAppsTable() - + "(app_id, created_at_time)" + " VALUES(?, ?) ON CONFLICT DO NOTHING"; - update(sqlCon, QUERY, pst -> { - pst.setString(1, tenantIdentifier.getAppId()); - pst.setLong(2, currentTime); - }); - } - - { - String QUERY = "INSERT INTO " + getConfig(start).getTenantsTable() - + "(app_id, tenant_id, created_at_time)" + " VALUES(?, ?, ?)"; - - update(sqlCon, QUERY, pst -> { - pst.setString(1, tenantIdentifier.getAppId()); - pst.setString(2, tenantIdentifier.getTenantId()); - pst.setLong(3, currentTime); - }); - } - + addTenantIdInTargetStorage_Transaction(start, sqlCon, tenantIdentifier); sqlCon.commit(); } catch (SQLException throwables) { throw new StorageTransactionLogicException(throwables); @@ -313,6 +289,48 @@ public static void addTenantIdInTargetStorage(Start start, TenantIdentifier tena } } + public static void addTenantIdInTargetStorage_Transaction(Start start, Connection con, + TenantIdentifier tenantIdentifier) throws + SQLException, StorageQueryException { + { + if (Start.isTesting && simulateErrorInAddingTenantIdInTargetStorage_forTesting) { + String QUERY = "SELECT 1 FROM " + getConfig(start).getAppsTable() + " WHERE app_id = ?"; + int val = execute(con, QUERY, pst -> { + pst.setString(1, tenantIdentifier.getAppId()); + }, rs -> { + if (rs.next()) { + return rs.getInt(1); + } + return -1; + }); + if (val == -1) { + throw new SQLException("Simulated error in addTenantIdInTargetStorage"); + } + } + + long currentTime = System.currentTimeMillis(); + { + String QUERY = "INSERT INTO " + getConfig(start).getAppsTable() + + "(app_id, created_at_time)" + " VALUES(?, ?) ON CONFLICT DO NOTHING"; + update(con, QUERY, pst -> { + pst.setString(1, tenantIdentifier.getAppId()); + pst.setLong(2, currentTime); + }); + } + + { + String QUERY = "INSERT INTO " + getConfig(start).getTenantsTable() + + "(app_id, tenant_id, created_at_time)" + " VALUES(?, ?, ?)"; + + update(con, QUERY, pst -> { + pst.setString(1, tenantIdentifier.getAppId()); + pst.setString(2, tenantIdentifier.getTenantId()); + pst.setLong(3, currentTime); + }); + } + } + } + public static void deleteTenantIdInTargetStorage(Start start, TenantIdentifier tenantIdentifier) throws StorageQueryException { try { diff --git a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java index 293cf6d2..dc9e5b7b 100644 --- a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java +++ b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java @@ -31,6 +31,7 @@ import io.supertokens.pluginInterface.exceptions.StorageQueryException; import io.supertokens.pluginInterface.multitenancy.*; import io.supertokens.pluginInterface.multitenancy.exceptions.TenantOrAppNotFoundException; +import io.supertokens.session.Session; import io.supertokens.storage.postgresql.Start; import io.supertokens.storage.postgresql.queries.MultitenancyQueries; import io.supertokens.storage.postgresql.test.TestingProcessManager; @@ -232,6 +233,63 @@ public void testThatCoreDoesNotStartWithThereIsAnErrorDuringTenantCreation() thr tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); } + @Test + public void testThatCoreDoesNotStartWithThereIsAnErrorDuringTenantCreationForRootCUD() throws Exception { + JsonObject coreConfig = new JsonObject(); + StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess()) + .modifyConfigToAddANewUserPoolForTesting(coreConfig, 1); + + TenantIdentifier tenantIdentifier = new TenantIdentifier("127.0.0.1", null, null); + + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage_forTesting = true; + try { + Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig( + tenantIdentifier, + new EmailPasswordConfig(true), + new ThirdPartyConfig(true, null), + new PasswordlessConfig(true), + null, null, + coreConfig + ), false); + fail(); + } catch (StorageQueryException e) { + // ignore + assertTrue(e.getMessage().contains("Simulated error")); + } + + TenantConfig[] allTenants = MultitenancyHelper.getInstance(process.getProcess()).getAllTenants(); + assertEquals(2, allTenants.length); // should have the new CUD + + Start start = (Start) StorageLayer.getBaseStorage(process.getProcess()); + update(start, "DELETE FROM apps;", pst -> {}); + + process.kill(false); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); + + String[] args = {"../"}; + + this.process = TestingProcessManager.start(args); + FeatureFlagTestContent.getInstance(process.getProcess()) + .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{EE_FEATURES.MULTI_TENANCY}); + ProcessState.EventAndException initFailure = process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.INIT_FAILURE); + + assertTrue(initFailure.exception.getMessage().contains("Simulated error")); + + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage_forTesting = false; + + this.process = TestingProcessManager.start(args); + FeatureFlagTestContent.getInstance(process.getProcess()) + .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{EE_FEATURES.MULTI_TENANCY}); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED)); + + File errorLog = new File(Config.getConfig(process.getProcess()).getErrorLogPath(process.getProcess())); + + // this should succeed now + tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); + + Session.createNewSession(process.getProcess(), "userid", new JsonObject(), new JsonObject()); + } + @Test public void testThatTenantComesToLifeOnceTheTargetDbIsUpAfterCoreRestart() throws Exception { Start start = ((Start) StorageLayer.getBaseStorage(process.getProcess())); From 46e81f7d127533e50776db4da0bcb2bfb29b978c Mon Sep 17 00:00:00 2001 From: Sattvik Chakravarthy Date: Thu, 18 Apr 2024 17:20:07 +0530 Subject: [PATCH 10/12] fix: pr comments --- .../supertokens/storage/postgresql/Start.java | 12 +-- .../test/multitenancy/StorageLayerTest.java | 25 ++--- .../TestForNoCrashDuringStartup.java | 98 +++++++++++++++++++ 3 files changed, 114 insertions(+), 21 deletions(-) diff --git a/src/main/java/io/supertokens/storage/postgresql/Start.java b/src/main/java/io/supertokens/storage/postgresql/Start.java index f76ee735..82832bb5 100644 --- a/src/main/java/io/supertokens/storage/postgresql/Start.java +++ b/src/main/java/io/supertokens/storage/postgresql/Start.java @@ -126,7 +126,6 @@ public class Start boolean enabled = true; static Thread mainThread = Thread.currentThread(); private Thread shutdownHook; - private Set tenantIdentifiers = new HashSet<>(); private boolean isBaseTenant = false; @@ -216,7 +215,7 @@ public void stopLogging() { } @Override - public void initStorage(boolean shouldWait) throws DbInitException { + public void initStorage(boolean shouldWait, List tenantIdentifiers) throws DbInitException { if (ConnectionPool.isAlreadyInitialised(this)) { return; } @@ -232,7 +231,7 @@ public void initStorage(boolean shouldWait) throws DbInitException { } catch (SQLException e) { throw new StorageQueryException(e); } - for (TenantIdentifier tenantIdentifier : this.tenantIdentifiers) { + for (TenantIdentifier tenantIdentifier : tenantIdentifiers) { try { this.addTenantIdInTargetStorage_Transaction(con, tenantIdentifier); } catch (DuplicateTenantException e) { @@ -245,11 +244,6 @@ public void initStorage(boolean shouldWait) throws DbInitException { } } - @Override - public synchronized void addTenantIdentifier(TenantIdentifier tenantIdentifier) { - this.tenantIdentifiers.add(tenantIdentifier); - } - @Override public T startTransaction(TransactionLogic logic) throws StorageTransactionLogicException, StorageQueryException { @@ -480,7 +474,7 @@ public void deleteAllInformation() throws StorageQueryException { } ProcessState.getInstance(this).clear(); try { - initStorage(false); + initStorage(false, new ArrayList<>()); enabled = true; // Allow get connection to work, to delete the data GeneralQueries.deleteAllTables(this); diff --git a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/StorageLayerTest.java b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/StorageLayerTest.java index 77204865..85ea22af 100644 --- a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/StorageLayerTest.java +++ b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/StorageLayerTest.java @@ -49,6 +49,7 @@ import java.io.IOException; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ExecutorService; @@ -117,7 +118,7 @@ public void mergingTenantWithBaseConfigWorks() Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); assertNotSame(StorageLayer.getStorage(new TenantIdentifier("abc", null, null), process.getProcess()), StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess())); @@ -168,7 +169,7 @@ public void storageInstanceIsReusedAcrossTenants() Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); assertSame(StorageLayer.getStorage(new TenantIdentifier(null, "abc", null), process.getProcess()), StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess())); @@ -226,7 +227,7 @@ public void storageInstanceIsReusedAcrossTenantsComplex() Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); assertSame(StorageLayer.getStorage(new TenantIdentifier(null, "abc", null), process.getProcess()), StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess())); @@ -290,7 +291,7 @@ public void mergingTenantWithBaseConfigWithInvalidConfigThrowsErrorWorks() tenantConfig)}; Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); fail(); } catch (InvalidConfigException e) { assert (e.getMessage() @@ -324,7 +325,7 @@ public void mergingTenantWithBaseConfigWithConflictingConfigsThrowsError() tenantConfig)}; Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); fail(); } catch (InvalidConfigException e) { assertEquals(e.getMessage(), @@ -359,7 +360,7 @@ public void mergingDifferentConnectionPoolIdTenantWithBaseConfigWithConflictingC tenantConfig)}; Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); fail(); } catch (InvalidConfigException e) { assertEquals(e.getMessage(), @@ -395,7 +396,7 @@ public void mergingDifferentUserPoolIdTenantWithBaseConfigWithConflictingConfigs tenantConfig)}; Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); Assert.assertEquals(io.supertokens.storage.postgresql.config.Config.getConfig( (Start) StorageLayer.getStorage(new TenantIdentifier("abc", null, null), process.getProcess())) @@ -447,7 +448,7 @@ public void newStorageIsNotCreatedWhenSameTenantIsAdded() Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); assertSame(StorageLayer.getStorage(new TenantIdentifier(null, "abc", null), process.getProcess()), existingStorage); @@ -528,7 +529,7 @@ public void testDifferentWaysToGetConfigBasedOnConnectionURIAndTenantId() Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); Assert.assertEquals(io.supertokens.storage.postgresql.config.Config.getConfig( (Start) StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess())) @@ -590,7 +591,7 @@ public void differentUserPoolCreatedBasedOnSchemaInConnectionUri() tenantConfig)}; Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); Assert.assertEquals(io.supertokens.storage.postgresql.config.Config.getConfig( (Start) StorageLayer.getStorage(new TenantIdentifier("abc", null, null), process.getProcess())) @@ -633,7 +634,7 @@ public void multipleTenantsSameUserPoolAndConnectionPoolShouldWork() tenantConfig)}; Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); assertSame(StorageLayer.getStorage(new TenantIdentifier(null, "abc", null), process.getProcess()), StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess())); @@ -669,7 +670,7 @@ public void multipleTenantsSameUserPoolAndDifferentConnectionPoolShouldWork() tenantConfig)}; Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); assertNotSame(StorageLayer.getStorage(new TenantIdentifier(null, "abc", null), process.getProcess()), StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess())); diff --git a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java index dc9e5b7b..bb41204f 100644 --- a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java +++ b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java @@ -170,6 +170,56 @@ public void testThatTenantComesToLifeOnceTheTargetDbIsUp() throws Exception { tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); } + @Test + public void testFailureInAppCreation() throws Exception { + JsonObject coreConfig = new JsonObject(); + + TenantIdentifier tenantIdentifier = new TenantIdentifier(null, "a1", null); + + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage_forTesting = true; + try { + Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig( + tenantIdentifier, + new EmailPasswordConfig(true), + new ThirdPartyConfig(true, null), + new PasswordlessConfig(true), + null, null, + coreConfig + ), false); + fail(); + } catch (StorageQueryException e) { + // ignore + assertTrue(e.getMessage().contains("Simulated error")); + } + + TenantConfig[] allTenants = MultitenancyHelper.getInstance(process.getProcess()).getAllTenants(); + assertEquals(2, allTenants.length); // should have the new CUD + + try { + tpSignInUpAndGetResponse(new TenantIdentifier(null, "a1", null), "google", "googleid1", "test@example.com", + process.getProcess(), SemVer.v5_0); + fail(); + } catch (HttpResponseException e) { + // ignore + assertTrue(e.getMessage().contains("AppId or tenantId not found")); // retried creating tenant entry + } + + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage_forTesting = false; + + Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig( + tenantIdentifier, + new EmailPasswordConfig(true), + new ThirdPartyConfig(true, null), + new PasswordlessConfig(true), + null, null, + coreConfig + ), false); + + // this should succeed now + tpSignInUpAndGetResponse(new TenantIdentifier(null, "a1", null), "google", "googleid1", "test@example.com", + process.getProcess(), SemVer.v5_0); + } + @Test public void testThatCoreDoesNotStartWithThereIsAnErrorDuringTenantCreation() throws Exception { JsonObject coreConfig = new JsonObject(); @@ -290,6 +340,54 @@ public void testThatCoreDoesNotStartWithThereIsAnErrorDuringTenantCreationForRoo Session.createNewSession(process.getProcess(), "userid", new JsonObject(), new JsonObject()); } + @Test + public void testThatCoreDoesNotStartWithThereIsAnErrorDuringAppCreationOnBaseTenantStorage() throws Exception { + JsonObject coreConfig = new JsonObject(); + + TenantIdentifier tenantIdentifier = new TenantIdentifier(null, "a1", null); + + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage_forTesting = true; + try { + Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig( + tenantIdentifier, + new EmailPasswordConfig(true), + new ThirdPartyConfig(true, null), + new PasswordlessConfig(true), + null, null, + coreConfig + ), false); + fail(); + } catch (StorageQueryException e) { + // ignore + assertTrue(e.getMessage().contains("Simulated error")); + } + + TenantConfig[] allTenants = MultitenancyHelper.getInstance(process.getProcess()).getAllTenants(); + assertEquals(2, allTenants.length); // should have the new CUD + + Start start = (Start) StorageLayer.getBaseStorage(process.getProcess()); + update(start, "DELETE FROM apps WHERE app_id = ?;", pst -> { + pst.setString(1, "a1"); + }); + + process.kill(false); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); + + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage_forTesting = false; + + String[] args = {"../"}; + + this.process = TestingProcessManager.start(args); + FeatureFlagTestContent.getInstance(process.getProcess()) + .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{EE_FEATURES.MULTI_TENANCY}); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED)); + + // this should succeed now + tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); + + Session.createNewSession(process.getProcess(), "userid", new JsonObject(), new JsonObject()); + } + @Test public void testThatTenantComesToLifeOnceTheTargetDbIsUpAfterCoreRestart() throws Exception { Start start = ((Start) StorageLayer.getBaseStorage(process.getProcess())); From fdc3efac1573b6a04405887f78728d77d7ded1f1 Mon Sep 17 00:00:00 2001 From: Sattvik Chakravarthy Date: Thu, 18 Apr 2024 18:42:58 +0530 Subject: [PATCH 11/12] fix: pr comments --- .../test/multitenancy/StorageLayerTest.java | 24 +++++++++---------- .../TestForNoCrashDuringStartup.java | 11 +-------- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/StorageLayerTest.java b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/StorageLayerTest.java index 85ea22af..97a3fb6b 100644 --- a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/StorageLayerTest.java +++ b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/StorageLayerTest.java @@ -118,7 +118,7 @@ public void mergingTenantWithBaseConfigWorks() Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); assertNotSame(StorageLayer.getStorage(new TenantIdentifier("abc", null, null), process.getProcess()), StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess())); @@ -169,7 +169,7 @@ public void storageInstanceIsReusedAcrossTenants() Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); assertSame(StorageLayer.getStorage(new TenantIdentifier(null, "abc", null), process.getProcess()), StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess())); @@ -227,7 +227,7 @@ public void storageInstanceIsReusedAcrossTenantsComplex() Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); assertSame(StorageLayer.getStorage(new TenantIdentifier(null, "abc", null), process.getProcess()), StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess())); @@ -291,7 +291,7 @@ public void mergingTenantWithBaseConfigWithInvalidConfigThrowsErrorWorks() tenantConfig)}; Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); fail(); } catch (InvalidConfigException e) { assert (e.getMessage() @@ -325,7 +325,7 @@ public void mergingTenantWithBaseConfigWithConflictingConfigsThrowsError() tenantConfig)}; Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); fail(); } catch (InvalidConfigException e) { assertEquals(e.getMessage(), @@ -360,7 +360,7 @@ public void mergingDifferentConnectionPoolIdTenantWithBaseConfigWithConflictingC tenantConfig)}; Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); fail(); } catch (InvalidConfigException e) { assertEquals(e.getMessage(), @@ -396,7 +396,7 @@ public void mergingDifferentUserPoolIdTenantWithBaseConfigWithConflictingConfigs tenantConfig)}; Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); Assert.assertEquals(io.supertokens.storage.postgresql.config.Config.getConfig( (Start) StorageLayer.getStorage(new TenantIdentifier("abc", null, null), process.getProcess())) @@ -448,7 +448,7 @@ public void newStorageIsNotCreatedWhenSameTenantIsAdded() Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); assertSame(StorageLayer.getStorage(new TenantIdentifier(null, "abc", null), process.getProcess()), existingStorage); @@ -529,7 +529,7 @@ public void testDifferentWaysToGetConfigBasedOnConnectionURIAndTenantId() Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); Assert.assertEquals(io.supertokens.storage.postgresql.config.Config.getConfig( (Start) StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess())) @@ -591,7 +591,7 @@ public void differentUserPoolCreatedBasedOnSchemaInConnectionUri() tenantConfig)}; Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); Assert.assertEquals(io.supertokens.storage.postgresql.config.Config.getConfig( (Start) StorageLayer.getStorage(new TenantIdentifier("abc", null, null), process.getProcess())) @@ -634,7 +634,7 @@ public void multipleTenantsSameUserPoolAndConnectionPoolShouldWork() tenantConfig)}; Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); assertSame(StorageLayer.getStorage(new TenantIdentifier(null, "abc", null), process.getProcess()), StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess())); @@ -670,7 +670,7 @@ public void multipleTenantsSameUserPoolAndDifferentConnectionPoolShouldWork() tenantConfig)}; Config.loadAllTenantConfig(process.getProcess(), tenants); - StorageLayer.loadAllTenantStorage(process.getProcess(), tenants, List.of()); + StorageLayer.loadAllTenantStorage(process.getProcess(), tenants); assertNotSame(StorageLayer.getStorage(new TenantIdentifier(null, "abc", null), process.getProcess()), StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess())); diff --git a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java index bb41204f..9b0d8af1 100644 --- a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java +++ b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java @@ -318,21 +318,12 @@ public void testThatCoreDoesNotStartWithThereIsAnErrorDuringTenantCreationForRoo String[] args = {"../"}; - this.process = TestingProcessManager.start(args); - FeatureFlagTestContent.getInstance(process.getProcess()) - .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{EE_FEATURES.MULTI_TENANCY}); - ProcessState.EventAndException initFailure = process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.INIT_FAILURE); - - assertTrue(initFailure.exception.getMessage().contains("Simulated error")); - - MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage_forTesting = false; - this.process = TestingProcessManager.start(args); FeatureFlagTestContent.getInstance(process.getProcess()) .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{EE_FEATURES.MULTI_TENANCY}); assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED)); - File errorLog = new File(Config.getConfig(process.getProcess()).getErrorLogPath(process.getProcess())); + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage_forTesting = false; // this should succeed now tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); From 94a7974dbf4c16b288766bb37d9913c0dc2bf6d9 Mon Sep 17 00:00:00 2001 From: Sattvik Chakravarthy Date: Fri, 19 Apr 2024 10:29:08 +0530 Subject: [PATCH 12/12] fix: update tests --- .../TestForNoCrashDuringStartup.java | 161 +++++++++++++++--- 1 file changed, 133 insertions(+), 28 deletions(-) diff --git a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java index 9b0d8af1..b8635224 100644 --- a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java +++ b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestForNoCrashDuringStartup.java @@ -19,7 +19,6 @@ import com.google.gson.JsonObject; import io.supertokens.Main; import io.supertokens.ProcessState; -import io.supertokens.config.Config; import io.supertokens.featureflag.EE_FEATURES; import io.supertokens.featureflag.FeatureFlagTestContent; import io.supertokens.featureflag.exceptions.FeatureNotEnabledException; @@ -32,6 +31,7 @@ import io.supertokens.pluginInterface.multitenancy.*; import io.supertokens.pluginInterface.multitenancy.exceptions.TenantOrAppNotFoundException; import io.supertokens.session.Session; +import io.supertokens.session.accessToken.AccessToken; import io.supertokens.storage.postgresql.Start; import io.supertokens.storage.postgresql.queries.MultitenancyQueries; import io.supertokens.storage.postgresql.test.TestingProcessManager; @@ -46,10 +46,7 @@ import org.junit.Before; import org.junit.Test; -import java.io.File; import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.util.Scanner; import static org.junit.Assert.*; import static io.supertokens.storage.postgresql.QueryExecutorTemplate.update; @@ -83,7 +80,7 @@ public void beforeEach() throws InterruptedException, InvalidProviderConfigExcep } @Test - public void testFailureInCUDCreation() throws Exception { + public void testThatCUDRecoversWhenItFailsToAddEntryDuringCreation() throws Exception { JsonObject coreConfig = new JsonObject(); StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess()) .modifyConfigToAddANewUserPoolForTesting(coreConfig, 1); @@ -124,7 +121,7 @@ public void testFailureInCUDCreation() throws Exception { } @Test - public void testThatTenantComesToLifeOnceTheTargetDbIsUp() throws Exception { + public void testThatCUDRecoversWhenTheDbIsDownDuringCreationButDbComesUpLater() throws Exception { Start start = ((Start) StorageLayer.getBaseStorage(process.getProcess())); try { update(start, "DROP DATABASE st5000;", pst -> {}); @@ -171,7 +168,7 @@ public void testThatTenantComesToLifeOnceTheTargetDbIsUp() throws Exception { } @Test - public void testFailureInAppCreation() throws Exception { + public void testThatAppRecoversAfterAppCreationFailedToAddEntry() throws Exception { JsonObject coreConfig = new JsonObject(); TenantIdentifier tenantIdentifier = new TenantIdentifier(null, "a1", null); @@ -221,7 +218,7 @@ public void testFailureInAppCreation() throws Exception { } @Test - public void testThatCoreDoesNotStartWithThereIsAnErrorDuringTenantCreation() throws Exception { + public void testThatCoreDoesNotCrashDuringStartupWhenCUDCreationFailedToAddEntryInTargetStorage() throws Exception { JsonObject coreConfig = new JsonObject(); StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess()) .modifyConfigToAddANewUserPoolForTesting(coreConfig, 1); @@ -260,23 +257,6 @@ public void testThatCoreDoesNotStartWithThereIsAnErrorDuringTenantCreation() thr allTenants = MultitenancyHelper.getInstance(process.getProcess()).getAllTenants(); assertEquals(2, allTenants.length); // should have the new CUD - File errorLog = new File(Config.getConfig(process.getProcess()).getErrorLogPath(process.getProcess())); - - boolean foundSimulatedError = false; - - try (Scanner scanner = new Scanner(errorLog, StandardCharsets.UTF_8)) { - while (scanner.hasNextLine()) { - String line = scanner.nextLine(); - System.out.println(line); - if (line.contains("Simulated error")) { - foundSimulatedError = true; - break; - } - } - } - - assertTrue(foundSimulatedError); - MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage_forTesting = false; // this should succeed now @@ -284,7 +264,7 @@ public void testThatCoreDoesNotStartWithThereIsAnErrorDuringTenantCreation() thr } @Test - public void testThatCoreDoesNotStartWithThereIsAnErrorDuringTenantCreationForRootCUD() throws Exception { + public void testThatCoreDoesNotCrashDuringStartupWhenTenantEntryIsInconsistentInTheBaseTenant() throws Exception { JsonObject coreConfig = new JsonObject(); StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess()) .modifyConfigToAddANewUserPoolForTesting(coreConfig, 1); @@ -332,7 +312,7 @@ public void testThatCoreDoesNotStartWithThereIsAnErrorDuringTenantCreationForRoo } @Test - public void testThatCoreDoesNotStartWithThereIsAnErrorDuringAppCreationOnBaseTenantStorage() throws Exception { + public void testThatCoreDoesNotCrashDuringStartupWhenAppCreationFailedToAddEntryInTheBaseTenantStorage() throws Exception { JsonObject coreConfig = new JsonObject(); TenantIdentifier tenantIdentifier = new TenantIdentifier(null, "a1", null); @@ -376,7 +356,132 @@ public void testThatCoreDoesNotStartWithThereIsAnErrorDuringAppCreationOnBaseTen // this should succeed now tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); - Session.createNewSession(process.getProcess(), "userid", new JsonObject(), new JsonObject()); + Session.createNewSession( + new TenantIdentifier(null, "a1", null), + StorageLayer.getBaseStorage(process.getProcess()), + process.getProcess(), "userid", new JsonObject(), new JsonObject(), true, + AccessToken.getLatestVersion(), false); + } + + @Test + public void testThatCoreDoesNotCrashDuringStartupWhenCUDCreationFailedToAddTenantEntryInTargetStorageWithLoadOnlyCUDConfig() throws Exception { + JsonObject coreConfig = new JsonObject(); + + TenantIdentifier tenantIdentifier = new TenantIdentifier("127.0.0.1", null, null); + + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage_forTesting = true; + try { + StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess()) + .modifyConfigToAddANewUserPoolForTesting(coreConfig, 1); + Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig( + tenantIdentifier, + new EmailPasswordConfig(true), + new ThirdPartyConfig(true, null), + new PasswordlessConfig(true), + null, null, + coreConfig + ), false); + fail(); + } catch (StorageQueryException e) { + // ignore + assertTrue(e.getMessage().contains("Simulated error")); + } + + try { + StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess()) + .modifyConfigToAddANewUserPoolForTesting(coreConfig, 2); + Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig( + new TenantIdentifier("localhost", null, null), + new EmailPasswordConfig(true), + new ThirdPartyConfig(true, null), + new PasswordlessConfig(true), + null, null, + coreConfig + ), false); + fail(); + } catch (StorageQueryException e) { + // ignore + assertTrue(e.getMessage().contains("Simulated error")); + } + + try { + StorageLayer.getStorage(new TenantIdentifier(null, null, null), process.getProcess()) + .modifyConfigToAddANewUserPoolForTesting(coreConfig, 3); + Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig( + new TenantIdentifier("cud2", null, null), + new EmailPasswordConfig(true), + new ThirdPartyConfig(true, null), + new PasswordlessConfig(true), + null, null, + coreConfig + ), false); + fail(); + } catch (StorageQueryException e) { + // ignore + assertTrue(e.getMessage().contains("Simulated error")); + } + + TenantConfig[] allTenants = MultitenancyHelper.getInstance(process.getProcess()).getAllTenants(); + assertEquals(4, allTenants.length); // should have the new CUD + + process.kill(false); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); + + MultitenancyQueries.simulateErrorInAddingTenantIdInTargetStorage_forTesting = false; + + String[] args = {"../"}; + + this.process = TestingProcessManager.start(args, false); + Utils.setValueInConfig("supertokens_saas_load_only_cud", "127.0.0.1:3567"); + FeatureFlagTestContent.getInstance(process.getProcess()) + .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{EE_FEATURES.MULTI_TENANCY}); + this.process.startProcess(); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED)); + + allTenants = MultitenancyHelper.getInstance(process.getProcess()).getAllTenants(); + assertEquals(2, allTenants.length); // should have the new CUD + + // this should succeed now + tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); + + try { + tpSignInUpAndGetResponse(new TenantIdentifier("localhost", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); + fail(); + } catch (HttpResponseException e) { + // ignore + } + + process.kill(false); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); + + this.process = TestingProcessManager.start(args, false); + Utils.setValueInConfig("supertokens_saas_load_only_cud", "localhost:3567"); + FeatureFlagTestContent.getInstance(process.getProcess()) + .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{EE_FEATURES.MULTI_TENANCY}); + this.process.startProcess(); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED)); + + allTenants = MultitenancyHelper.getInstance(process.getProcess()).getAllTenants(); + assertEquals(2, allTenants.length); // should have the new CUD + + // this should succeed now + tpSignInUpAndGetResponse(new TenantIdentifier("localhost", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); + try { + tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "test@example.com", process.getProcess(), SemVer.v5_0); + fail(); + } catch (HttpResponseException e) { + // ignore + } + + process.kill(false); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); + + this.process = TestingProcessManager.start(args, false); + Utils.setValueInConfig("supertokens_saas_load_only_cud", null); + FeatureFlagTestContent.getInstance(process.getProcess()) + .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{EE_FEATURES.MULTI_TENANCY}); + this.process.startProcess(); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED)); } @Test