Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: resource reloading #673

Merged
merged 8 commits into from
May 12, 2023
Merged

fix: resource reloading #673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d7112c8
fix: resource reloading
sattvikc May 10, 2023
2ae3395
fix: license test task reloading
sattvikc May 10, 2023
a4acd26
fix: reload revert
sattvikc May 10, 2023
ac71ba7
fix: impl
sattvikc May 11, 2023
4e6e777
fix: pr comments
sattvikc May 11, 2023
f6ed47f
fix: pr comments
sattvikc May 11, 2023
db6fea9
fix: pr comment
sattvikc May 12, 2023
b2ab0c4
fix: pr comments
sattvikc May 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ee/src/main/java/io/supertokens/ee/EEFeatureFlag.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,11 @@ public void updateEnabledFeaturesValueReadFromDbTime(long newTime) {
public void constructor(Main main, AppIdentifier appIdentifier) {
this.main = main;
this.appIdentifier = appIdentifier;
Cronjobs.addCronjob(main, EELicenseCheck.getInstance(main, this.appIdentifier.getAsPublicTenantIdentifier()));

// EELicenseCheck.init does not create a new CronTask each time, it creates for the first time and
// returns the same instance from there on.
Cronjobs.addCronjob(main, EELicenseCheck.init(main, StorageLayer.getTenantsWithUniqueUserPoolId(main)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a comment above explaining that it doesn't add per constructor call even though it looks like that

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


try {
this.syncFeatureFlagWithLicenseKey();
} catch (HttpResponseException | IOException e) {
Expand Down
29 changes: 14 additions & 15 deletions ee/src/main/java/io/supertokens/ee/cronjobs/EELicenseCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,33 @@
import io.supertokens.Main;
import io.supertokens.cronjobs.CronTask;
import io.supertokens.cronjobs.CronTaskTest;
import io.supertokens.cronjobs.Cronjobs;
import io.supertokens.cronjobs.deleteExpiredAccessTokenSigningKeys.DeleteExpiredAccessTokenSigningKeys;
import io.supertokens.ee.EEFeatureFlag;
import io.supertokens.featureflag.FeatureFlag;
import io.supertokens.pluginInterface.multitenancy.AppIdentifier;
import io.supertokens.pluginInterface.multitenancy.TenantIdentifier;
import io.supertokens.pluginInterface.multitenancy.exceptions.TenantOrAppNotFoundException;
import io.supertokens.storageLayer.StorageLayer;

import java.util.List;

public class EELicenseCheck extends CronTask {

public static final String RESOURCE_KEY = "io.supertokens.ee.cronjobs.EELicenseCheck";

private EELicenseCheck(Main main, TenantIdentifier targetTenant) {
super("EELicenseCheck", main, targetTenant);
private EELicenseCheck(Main main, List<List<TenantIdentifier>> tenantsInfo) {
super("EELicenseCheck", main, tenantsInfo, true);
}
rishabhpoddar marked this conversation as resolved.
Show resolved Hide resolved

public static EELicenseCheck getInstance(Main main, TenantIdentifier tenantIdentifier) {
try {
return (EELicenseCheck) main.getResourceDistributor()
.getResource(tenantIdentifier, RESOURCE_KEY);
} catch (TenantOrAppNotFoundException e) {
return (EELicenseCheck) main.getResourceDistributor()
.setResource(tenantIdentifier, RESOURCE_KEY,
new EELicenseCheck(main, tenantIdentifier));
}
public static EELicenseCheck init(Main main, List<List<TenantIdentifier>> tenantsInfo) {
return (EELicenseCheck) main.getResourceDistributor()
.setResource(new TenantIdentifier(null, null, null), RESOURCE_KEY,
new EELicenseCheck(main, tenantsInfo));
}

@Override
protected void doTaskForTargetTenant(TenantIdentifier tenantIdentifier) throws Exception {
// this cronjob is for one tenant only (the targetTenant provided in the constructor)
FeatureFlag.getInstance(main, tenantIdentifier.toAppIdentifier()).syncFeatureFlagWithLicenseKey();
protected void doTaskPerApp(AppIdentifier app) throws Exception {
FeatureFlag.getInstance(main, app).syncFeatureFlagWithLicenseKey();
}

@Override
Expand Down
7 changes: 4 additions & 3 deletions src/main/java/io/supertokens/Main.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import java.lang.reflect.Field;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;

Expand Down Expand Up @@ -205,7 +206,7 @@ private void init() throws IOException, StorageQueryException {

try {
// load all configs for each of the tenants.
MultitenancyHelper.getInstance(this).loadConfig();
MultitenancyHelper.getInstance(this).loadConfig(new ArrayList<>());

// init storage layers for each unique db connection based on unique (user pool ID, connection pool ID).
MultitenancyHelper.getInstance(this).loadStorageLayer();
Expand All @@ -214,11 +215,11 @@ private void init() throws IOException, StorageQueryException {
}

// load feature flag for all loaded apps
MultitenancyHelper.getInstance(this).loadFeatureFlag();
MultitenancyHelper.getInstance(this).loadFeatureFlag(new ArrayList<>());

// init signing keys
try {
MultitenancyHelper.getInstance(this).loadSigningKeys();
MultitenancyHelper.getInstance(this).loadSigningKeys(new ArrayList<>());
} catch (UnsupportedJWTSigningAlgorithmException e) {
throw new QuitProgramException(e);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/io/supertokens/ProcessState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public enum PROCESS_STATE {
PASSWORD_HASH_BCRYPT, PASSWORD_HASH_ARGON, PASSWORD_VERIFY_BCRYPT, PASSWORD_VERIFY_ARGON,
PASSWORD_VERIFY_FIREBASE_SCRYPT, ADDING_REMOTE_ADDRESS_FILTER, LICENSE_KEY_CHECK_NETWORK_CALL,
INVALID_LICENSE_KEY, SERVER_ERROR_DURING_LICENSE_KEY_CHECK_FAIL, LOADING_ALL_TENANT_CONFIG,
LOADING_ALL_TENANT_STORAGE
LOADING_ALL_TENANT_STORAGE, TENANTS_CHANGED_DURING_REFRESH_FROM_DB
}

public static class EventAndException {
Expand Down
25 changes: 20 additions & 5 deletions src/main/java/io/supertokens/ResourceDistributor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public synchronized SingletonResource getResource(TenantIdentifier tenantIdentif
throw new TenantOrAppNotFoundException(tenantIdentifier);
}

MultitenancyHelper.getInstance(main).refreshTenantsInCoreIfRequired(true);
MultitenancyHelper.getInstance(main).refreshTenantsInCoreBasedOnChangesInCoreConfigOrIfTenantListChanged(true);

// we try again..
resource = resources.get(new KeyClass(tenantIdentifier, key));
Expand Down Expand Up @@ -104,12 +104,27 @@ public synchronized SingletonResource setResource(TenantIdentifier tenantIdentif
return resource;
}

public synchronized SingletonResource removeResource(TenantIdentifier tenantIdentifier,
@Nonnull String key) {
SingletonResource singletonResource = resources.get(new KeyClass(tenantIdentifier, key));
if (singletonResource == null) {
return null;
}
resources.remove(new KeyClass(tenantIdentifier, key));
return singletonResource;
}

public synchronized SingletonResource setResource(AppIdentifier appIdentifier,
@Nonnull String key,
SingletonResource resource) {
return setResource(appIdentifier.getAsPublicTenantIdentifier(), key, resource);
}

public synchronized SingletonResource removeResource(AppIdentifier appIdentifier,
@Nonnull String key) {
return removeResource(appIdentifier.getAsPublicTenantIdentifier(), key);
}

public synchronized void clearAllResourcesWithResourceKey(String inputKey) {
List<KeyClass> toRemove = new ArrayList<>();
resources.forEach((key, value) -> {
Expand Down Expand Up @@ -138,12 +153,12 @@ public synchronized SingletonResource setResource(@Nonnull String key,
return setResource(new TenantIdentifier(null, null, null), key, resource);
}

public interface Func {
void performTask() throws FuncException;
public interface Func<T> {
T performTask() throws FuncException;
}

public synchronized void withResourceDistributorLock(Func func) throws FuncException {
func.performTask();
public synchronized <T> T withResourceDistributorLock(Func<T> func) throws FuncException {
return func.performTask();
}

public interface FuncWithReturn<T> {
Expand Down
36 changes: 29 additions & 7 deletions src/main/java/io/supertokens/config/Config.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import io.supertokens.ResourceDistributor;
import io.supertokens.cliOptions.CLIOptions;
import io.supertokens.output.Logging;
import io.supertokens.pluginInterface.LOG_LEVEL;
import io.supertokens.pluginInterface.Storage;
import io.supertokens.pluginInterface.exceptions.InvalidConfigException;
import io.supertokens.pluginInterface.multitenancy.TenantConfig;
Expand All @@ -36,9 +35,10 @@

import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;

public class Config extends ResourceDistributor.SingletonResource {

Expand All @@ -62,7 +62,7 @@ private Config(Main main, JsonObject jsonConfig) throws IOException, InvalidConf
this.core = config;
}

private static Config getInstance(TenantIdentifier tenantIdentifier, Main main)
public static Config getInstance(TenantIdentifier tenantIdentifier, Main main)
throws TenantOrAppNotFoundException {
return (Config) main.getResourceDistributor().getResource(tenantIdentifier, RESOURCE_KEY);
}
Expand Down Expand Up @@ -98,8 +98,14 @@ private static String getConfigFilePath(Main main) {
: CLIOptions.get(main).getConfigFilePath();
}

@TestOnly
public static void loadAllTenantConfig(Main main, TenantConfig[] tenants)
throws IOException, InvalidConfigException {
loadAllTenantConfig(main, tenants, new ArrayList<>());
}

public static void loadAllTenantConfig(Main main, TenantConfig[] tenants, List<TenantIdentifier> tenantsThatChanged)
throws IOException, InvalidConfigException {
ProcessState.getInstance(main).addState(ProcessState.PROCESS_STATE.LOADING_ALL_TENANT_CONFIG, null);
Map<ResourceDistributor.KeyClass, JsonObject> normalisedConfigs = getNormalisedConfigsForAllTenants(
tenants,
Expand All @@ -110,16 +116,32 @@ public static void loadAllTenantConfig(Main main, TenantConfig[] tenants)
// At this point, we know that all configs are valid.
rishabhpoddar marked this conversation as resolved.
Show resolved Hide resolved
try {
main.getResourceDistributor().withResourceDistributorLock(() -> {
main.getResourceDistributor().clearAllResourcesWithResourceKey(RESOURCE_KEY);
try {
Map<ResourceDistributor.KeyClass, ResourceDistributor.SingletonResource> existingResources =
main.getResourceDistributor()
.getAllResourcesWithResourceKey(RESOURCE_KEY);
main.getResourceDistributor().clearAllResourcesWithResourceKey(RESOURCE_KEY);
for (ResourceDistributor.KeyClass key : normalisedConfigs.keySet()) {
main.getResourceDistributor()
.setResource(key.getTenantIdentifier(), RESOURCE_KEY,
new Config(main, normalisedConfigs.get(key)));
ResourceDistributor.SingletonResource resource = existingResources.get(
new ResourceDistributor.KeyClass(
key.getTenantIdentifier(),
RESOURCE_KEY));
if (resource != null && !tenantsThatChanged.contains(key.getTenantIdentifier())) {
main.getResourceDistributor()
.setResource(key.getTenantIdentifier(),
RESOURCE_KEY,
resource);
} else {
main.getResourceDistributor()
.setResource(key.getTenantIdentifier(), RESOURCE_KEY,
new Config(main, normalisedConfigs.get(key)));

}
}
} catch (InvalidConfigException | IOException e) {
throw new ResourceDistributor.FuncException(e);
}
return null;
});
} catch (ResourceDistributor.FuncException e) {
if (e.getCause() instanceof InvalidConfigException) {
Expand Down
9 changes: 8 additions & 1 deletion src/main/java/io/supertokens/cronjobs/Cronjobs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import io.supertokens.ResourceDistributor;
import io.supertokens.pluginInterface.multitenancy.TenantIdentifier;
import io.supertokens.pluginInterface.multitenancy.exceptions.TenantOrAppNotFoundException;
import org.jetbrains.annotations.TestOnly;

import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -92,8 +93,14 @@ public static void addCronjob(Main main, CronTask task) {
synchronized (instance.lock) {
instance.executor.scheduleWithFixedDelay(task, task.getInitialWaitTimeSeconds(),
task.getIntervalTimeSeconds(), TimeUnit.SECONDS);
instance.tasks.add(task);
if (!instance.tasks.contains(task)) {
instance.tasks.add(task);
}
Comment on lines +96 to +98
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test for this

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}
}

@TestOnly
public List<CronTask> getTasks() {
return this.tasks;
}
}
3 changes: 0 additions & 3 deletions src/main/java/io/supertokens/emailpassword/EmailPassword.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ public static long getPasswordResetTokenLifetimeForTests(Main main) {

private static long getPasswordResetTokenLifetime(TenantIdentifier tenantIdentifier, Main main)
throws TenantOrAppNotFoundException {
if (Main.isTesting) {
return EmailPasswordTest.getInstance(main).getPasswordResetTokenLifetime();
}
return Config.getConfig(tenantIdentifier, main).getPasswordResetTokenLifetime();
}

Expand Down
3 changes: 0 additions & 3 deletions src/main/java/io/supertokens/emailverification/EmailVerification.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ public static long getEmailVerificationTokenLifetimeForTests(Main main) {

private static long getEmailVerificationTokenLifetime(TenantIdentifier tenantIdentifier, Main main)
throws TenantOrAppNotFoundException {
if (Main.isTesting) {
return EmailVerificationTest.getInstance(main).getEmailVerificationTokenLifetime();
}
return Config.getConfig(tenantIdentifier, main).getEmailVerificationTokenLifetime();
}

Expand Down
7 changes: 4 additions & 3 deletions src/main/java/io/supertokens/featureflag/FeatureFlag.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public static void initForBaseTenant(Main main, String eeFolderPath) throws Malf
new FeatureFlag(main, eeFolderPath));
}

public static void loadForAllTenants(Main main, List<AppIdentifier> apps) {
public static void loadForAllTenants(Main main, List<AppIdentifier> apps, List<TenantIdentifier> tenantsThatChanged) {
try {
main.getResourceDistributor().withResourceDistributorLock(() -> {
rishabhpoddar marked this conversation as resolved.
Show resolved Hide resolved
Map<ResourceDistributor.KeyClass, ResourceDistributor.SingletonResource> existingResources =
Expand All @@ -134,7 +134,7 @@ public static void loadForAllTenants(Main main, List<AppIdentifier> apps) {
new ResourceDistributor.KeyClass(
app,
RESOURCE_KEY));
if (resource != null) {
if (resource != null && !tenantsThatChanged.contains(app.getAsPublicTenantIdentifier())) {
main.getResourceDistributor()
.setResource(app,
RESOURCE_KEY,
Expand All @@ -147,6 +147,7 @@ public static void loadForAllTenants(Main main, List<AppIdentifier> apps) {
new FeatureFlag(main, app));
}
}
return null;
});
} catch (ResourceDistributor.FuncException e) {
throw new RuntimeException(e);
Expand Down Expand Up @@ -217,4 +218,4 @@ public static void clearURLClassLoader() {
FeatureFlag.ucl = null;
}
}
}
}
Loading