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.

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

feat: Moved Tenant Information to Redis for quick access #33309

Merged
merged 20 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
63a005d
Merge branch 'release' of github.com:appsmithorg/appsmith into release
NilanshBansal May 6, 2024
60dd0f9
Merge branch 'release' of github.com:appsmithorg/appsmith into release
NilanshBansal May 8, 2024
9b144b6
initial setup
NilanshBansal May 9, 2024
9f37234
feat: added tenant configuration to redis cache (#33083)
NilanshBansal May 9, 2024
e889d83
code optimised to replace usage of tenantRepository to cached function
NilanshBansal May 9, 2024
b2c301e
fixed cyclic dependency
NilanshBansal May 9, 2024
883b05a
Update app/server/appsmith-server/src/main/java/com/appsmith/server/s…
NilanshBansal May 10, 2024
064d0b1
cache evicted after db update to avoid inconsistency
NilanshBansal May 10, 2024
0702551
Merge remote-tracking branch 'origin/feature/issue-33083/tenant-confi…
NilanshBansal May 10, 2024
9543f94
fixed tests
NilanshBansal May 13, 2024
958f5e2
added implements serializable for tenant configuration
NilanshBansal May 13, 2024
d469047
addressed review comments and other optimisations
NilanshBansal May 13, 2024
2d1115d
updated cacheName and added notDeleted criteria
NilanshBansal May 14, 2024
bdae9ad
Merge branch 'release' of github.com:appsmithorg/appsmith into featur…
NilanshBansal May 14, 2024
4d1deed
cache evicted from update func
NilanshBansal May 14, 2024
da5af58
moved cache invalidation to repository layer
NilanshBansal May 14, 2024
1910c61
Revert "moved cache invalidation to repository layer"
NilanshBansal May 14, 2024
371f2d1
optimisations
NilanshBansal May 14, 2024
bb21eae
code rabbit suggestion
NilanshBansal May 14, 2024
306e77f
Merge branch 'release' of github.com:appsmithorg/appsmith into featur…
NilanshBansal May 14, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@
import org.springframework.data.annotation.Transient;
import org.springframework.data.mongodb.core.mapping.Document;

import java.io.Serializable;

@Getter
@Setter
@ToString
@NoArgsConstructor
@Document
@FieldNameConstants
public class Tenant extends BaseDomain {
public class Tenant extends BaseDomain implements Serializable {

@Unique String slug;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import lombok.Data;
import lombok.EqualsAndHashCode;

import java.io.Serializable;

@Data
@EqualsAndHashCode(callSuper = true)
public class TenantConfiguration extends TenantConfigurationCE {}
public class TenantConfiguration extends TenantConfigurationCE implements Serializable {}
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@
import lombok.Data;
import org.apache.commons.lang3.ObjectUtils;

import java.io.Serializable;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;

@Data
public class TenantConfigurationCE {
public class TenantConfigurationCE implements Serializable {

private String googleMapsKey;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.appsmith.server.repositories.ce;

import com.appsmith.server.domains.Tenant;
import com.appsmith.server.domains.User;
import reactor.core.publisher.Mono;

Expand All @@ -18,4 +19,8 @@ public interface CacheableRepositoryHelperCE {
Mono<String> getDefaultTenantId();

Mono<String> getInstanceAdminPermissionGroupId();

Mono<Tenant> fetchDefaultTenant(String tenantId);

Mono<Void> evictCachedTenant(String tenantId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.appsmith.server.domains.Config;
import com.appsmith.server.domains.PermissionGroup;
import com.appsmith.server.domains.Tenant;
import com.appsmith.server.domains.TenantConfiguration;
import com.appsmith.server.domains.User;
import com.appsmith.server.domains.Workspace;
import com.appsmith.server.exceptions.AppsmithError;
Expand Down Expand Up @@ -166,4 +167,33 @@ public Mono<String> getInstanceAdminPermissionGroupId() {
.doOnSuccess(permissionGroupId ->
inMemoryCacheableRepositoryHelper.setInstanceAdminPermissionGroupId(permissionGroupId));
}

/**
* Returns the default tenant from the cache if present.
* If not present in cache, then it fetches the default tenant from the database and adds to redis.
* @param tenantId
* @return
*/
@Cache(cacheName = "tenant", key = "{#tenantId}")
@Override
public Mono<Tenant> fetchDefaultTenant(String tenantId) {
BridgeQuery<Tenant> defaultTenantCriteria = Bridge.equal(Tenant.Fields.slug, FieldName.DEFAULT);
BridgeQuery<Tenant> notDeletedCriteria = notDeleted();
BridgeQuery<Tenant> andCriteria = Bridge.and(defaultTenantCriteria, notDeletedCriteria);
Query query = new Query();
query.addCriteria(andCriteria);

return mongoOperations.findOne(query, Tenant.class).map(tenant -> {
if (tenant.getTenantConfiguration() == null) {
tenant.setTenantConfiguration(new TenantConfiguration());
}
return tenant;
});
}

@CacheEvict(cacheName = "tenant", key = "{#tenantId}")
@Override
public Mono<Void> evictCachedTenant(String tenantId) {
return Mono.empty().then();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import reactor.core.publisher.Mono;

public interface TenantRepositoryCE extends BaseRepository<Tenant, String>, CustomTenantRepositoryCE {

// Use tenantService.getDefaultTenant() instead of this method as it is cached to redis.
NilanshBansal marked this conversation as resolved.
Show resolved Hide resolved
@Deprecated(forRemoval = true)
Mono<Tenant> findBySlug(String slug);
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.appsmith.server.services;

import com.appsmith.server.helpers.FeatureFlagMigrationHelper;
import com.appsmith.server.repositories.CacheableRepositoryHelper;
import com.appsmith.server.repositories.TenantRepository;
import com.appsmith.server.services.ce.TenantServiceCEImpl;
import com.appsmith.server.solutions.EnvManager;
Expand All @@ -19,7 +20,15 @@ public TenantServiceImpl(
AnalyticsService analyticsService,
ConfigService configService,
@Lazy EnvManager envManager,
FeatureFlagMigrationHelper featureFlagMigrationHelper) {
super(validator, repository, analyticsService, configService, envManager, featureFlagMigrationHelper);
FeatureFlagMigrationHelper featureFlagMigrationHelper,
CacheableRepositoryHelper cacheableRepositoryHelper) {
super(
validator,
repository,
analyticsService,
configService,
envManager,
featureFlagMigrationHelper,
cacheableRepositoryHelper);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
@Slf4j
@RequiredArgsConstructor
public class CacheableFeatureFlagHelperCEImpl implements CacheableFeatureFlagHelperCE {

private final TenantRepository tenantRepository;
private final ConfigService configService;
private final CloudServicesConfig cloudServicesConfig;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.helpers.CollectionUtils;
import com.appsmith.server.helpers.FeatureFlagMigrationHelper;
import com.appsmith.server.repositories.CacheableRepositoryHelper;
import com.appsmith.server.repositories.TenantRepository;
import com.appsmith.server.services.AnalyticsService;
import com.appsmith.server.services.BaseService;
Expand Down Expand Up @@ -39,17 +40,21 @@ public class TenantServiceCEImpl extends BaseService<TenantRepository, Tenant, S

private final FeatureFlagMigrationHelper featureFlagMigrationHelper;

private final CacheableRepositoryHelper cacheableRepositoryHelper;

public TenantServiceCEImpl(
Validator validator,
TenantRepository repository,
AnalyticsService analyticsService,
ConfigService configService,
@Lazy EnvManager envManager,
FeatureFlagMigrationHelper featureFlagMigrationHelper) {
FeatureFlagMigrationHelper featureFlagMigrationHelper,
CacheableRepositoryHelper cacheableRepositoryHelper) {
super(validator, repository, analyticsService);
this.configService = configService;
this.envManager = envManager;
this.featureFlagMigrationHelper = featureFlagMigrationHelper;
this.cacheableRepositoryHelper = cacheableRepositoryHelper;
}

@Override
Expand All @@ -59,7 +64,6 @@ public Mono<String> getDefaultTenantId() {
if (StringUtils.hasLength(tenantId)) {
return Mono.just(tenantId);
}

return repository.findBySlug(FieldName.DEFAULT).map(Tenant::getId).map(tenantId -> {
// Set the cache value before returning.
this.tenantId = tenantId;
Expand All @@ -69,6 +73,7 @@ public Mono<String> getDefaultTenantId() {

@Override
public Mono<Tenant> updateTenantConfiguration(String tenantId, TenantConfiguration tenantConfiguration) {
Mono<Void> evictTenantCache = cacheableRepositoryHelper.evictCachedTenant(tenantId);
return repository
.findById(tenantId, MANAGE_TENANT)
.switchIfEmpty(Mono.error(
Expand All @@ -89,14 +94,23 @@ public Mono<Tenant> updateTenantConfiguration(String tenantId, TenantConfigurati
return Mono.empty();
});
}

return envMono.then(Mono.zip(Mono.just(oldtenantConfiguration), Mono.just(tenant)));
})
.flatMap(tuple2 -> {
Tenant tenant = tuple2.getT2();
TenantConfiguration oldConfig = tuple2.getT1();
AppsmithBeanUtils.copyNestedNonNullProperties(tenantConfiguration, oldConfig);
tenant.setTenantConfiguration(oldConfig);
return repository.updateById(tenantId, tenant, MANAGE_TENANT);
Mono<Tenant> updatedTenantMono = repository
.updateById(tenantId, tenant, MANAGE_TENANT)
.cache();
// Firstly updating the Tenant object in the database and then evicting the cache.
// returning the updatedTenant, notice the updatedTenantMono is cached using .cache()
// hence it will not be evaluated again
return updatedTenantMono
.then(Mono.defer(() -> evictTenantCache))
.then(updatedTenantMono);
});
}

Expand Down Expand Up @@ -151,16 +165,9 @@ public Mono<Tenant> getTenantConfiguration() {

@Override
public Mono<Tenant> getDefaultTenant() {
// Get the default tenant object from the DB and then populate the relevant user permissions in that
// We are doing this differently because `findBySlug` is a Mongo JPA query and not a custom Appsmith query
return repository
.findBySlug(FieldName.DEFAULT)
.map(tenant -> {
if (tenant.getTenantConfiguration() == null) {
tenant.setTenantConfiguration(new TenantConfiguration());
}
return tenant;
})
// Fetching Tenant from redis cache
return getDefaultTenantId()
NilanshBansal marked this conversation as resolved.
Show resolved Hide resolved
.flatMap(tenantId -> cacheableRepositoryHelper.fetchDefaultTenant(tenantId))
.flatMap(tenant -> repository.setUserPermissionsInObject(tenant).switchIfEmpty(Mono.just(tenant)));
}

Expand Down Expand Up @@ -192,9 +199,12 @@ protected Mono<Tenant> getClientPertinentTenant(Tenant dbTenant, Tenant clientTe
return Mono.just(clientTenant);
}

// This function is used to save the tenant object in the database and evict the cache
@Override
public Mono<Tenant> save(Tenant tenant) {
return repository.save(tenant);
Mono<Void> evictTenantCache = cacheableRepositoryHelper.evictCachedTenant(tenantId);
Mono<Tenant> savedTenantMono = repository.save(tenant).cache();
return savedTenantMono.then(Mono.defer(() -> evictTenantCache)).then(savedTenantMono);
}

/**
Expand Down Expand Up @@ -242,6 +252,19 @@ public Mono<Tenant> retrieveById(String id) {
return repository.findById(id);
}

/**
* This function updates the tenant object in the database and evicts the cache
* @param tenantId
* @param tenant
* @return
*/
@Override
public Mono<Tenant> update(String tenantId, Tenant tenant) {
Mono<Void> evictTenantCache = cacheableRepositoryHelper.evictCachedTenant(tenantId);
Mono<Tenant> updatedTenantMono = super.update(tenantId, tenant).cache();
return updatedTenantMono.then(Mono.defer(() -> evictTenantCache)).then(updatedTenantMono);
}

/**
* This function checks if the tenant needs to be restarted and restarts after the feature flag migrations are
* executed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,11 @@ public void setup() throws IOException {

@AfterEach
public void cleanup() {
Tenant updatedTenant = new Tenant();
updatedTenant.setTenantConfiguration(originalTenantConfiguration);
tenantService
.getDefaultTenant()
.flatMap(tenant -> tenantRepository.updateAndReturn(
tenant.getId(),
Bridge.update().set(Tenant.Fields.tenantConfiguration, originalTenantConfiguration),
Optional.empty()))
.getDefaultTenantId()
.flatMap(tenantId -> tenantService.update(tenantId, updatedTenant))
NilanshBansal marked this conversation as resolved.
Show resolved Hide resolved
.block();
}

Expand Down