Skip to content

Commit

Permalink
fix: pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
sattvikc committed Apr 18, 2024
1 parent 2b91b35 commit 46e81f7
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 21 deletions.
12 changes: 3 additions & 9 deletions src/main/java/io/supertokens/storage/postgresql/Start.java
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ public class Start
boolean enabled = true;
static Thread mainThread = Thread.currentThread();
private Thread shutdownHook;
private Set<TenantIdentifier> tenantIdentifiers = new HashSet<>();

private boolean isBaseTenant = false;

Expand Down Expand Up @@ -216,7 +215,7 @@ public void stopLogging() {
}

@Override
public void initStorage(boolean shouldWait) throws DbInitException {
public void initStorage(boolean shouldWait, List<TenantIdentifier> tenantIdentifiers) throws DbInitException {
if (ConnectionPool.isAlreadyInitialised(this)) {
return;
}
Expand All @@ -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) {
Expand All @@ -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> T startTransaction(TransactionLogic<T> logic)
throws StorageTransactionLogicException, StorageQueryException {
Expand Down Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()));
Expand Down Expand Up @@ -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()));
Expand Down Expand Up @@ -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()));
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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()))
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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()))
Expand Down Expand Up @@ -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()))
Expand Down Expand Up @@ -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()));
Expand Down Expand Up @@ -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()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,56 @@ public void testThatTenantComesToLifeOnceTheTargetDbIsUp() throws Exception {
tpSignInUpAndGetResponse(new TenantIdentifier("127.0.0.1", null, null), "google", "googleid1", "[email protected]", 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", "[email protected]",
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", "[email protected]",
process.getProcess(), SemVer.v5_0);
}

@Test
public void testThatCoreDoesNotStartWithThereIsAnErrorDuringTenantCreation() throws Exception {
JsonObject coreConfig = new JsonObject();
Expand Down Expand Up @@ -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", "[email protected]", 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()));
Expand Down

0 comments on commit 46e81f7

Please sign in to comment.