diff --git a/CODEOWNERS b/CODEOWNERS index cb7f2daf14..7390a08867 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -39,6 +39,21 @@ app/client/src/ee/pages/Editor/NavigationSettings/LogoInput.tsx @dhruvikn app/client/src/ce/entities/FeatureFlag.ts @hetunandu app/client/src/ee/entities/FeatureFlag.ts @hetunandu app/server/appsmith-server/src/main/resources/features/init-flags.xml @hetunandu +app/server/appsmith-server/src/main/java/com/appsmith/server/featureflags/* @nilanshbansal +app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserIdentifierService.java @nilanshbansal +app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserIdentifierServiceImpl.java @nilanshbansal +app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserIdentifierServiceCE.java @nilanshbansal +app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserIdentifierServiceCEImpl.java @nilanshbansal +app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/UserIdentifierServiceCEImplTest.java @nilanshbansal +app/server/appsmith-server/src/main/java/com/appsmith/server/services/CacheableFeatureFlagHelper.java @nilanshbansal +app/server/appsmith-server/src/main/java/com/appsmith/server/services/CacheableFeatureFlagHelperImpl.java @nilanshbansal +app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/CacheableFeatureFlagHelperCE.java @nilanshbansal +app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/CacheableFeatureFlagHelperCEImpl.java @nilanshbansal +app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/FeatureFlagServiceCE.java @nilanshbansal +app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/FeatureFlagServiceCEImpl.java @nilanshbansal +app/server/appsmith-server/src/main/java/com/appsmith/server/services/FeatureFlagService.java @nilanshbansal +app/server/appsmith-server/src/main/java/com/appsmith/server/services/FeatureFlagServiceImpl.java @nilanshbansal +app/server/appsmith-server/src/test/java/com/appsmith/server/services/FeatureFlagServiceTest.java @nilanshbansal # UI Builders Pod app/client/generators/* @appsmithorg/ui-builders diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationSuccessHandlerCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationSuccessHandlerCE.java index 8b14269f2e..0f967234bc 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationSuccessHandlerCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/AuthenticationSuccessHandlerCE.java @@ -9,7 +9,6 @@ import com.appsmith.server.domains.Application; import com.appsmith.server.domains.LoginSource; import com.appsmith.server.domains.User; import com.appsmith.server.domains.Workspace; -import com.appsmith.server.featureflags.FeatureFlagTrait; import com.appsmith.server.helpers.RedirectHelper; import com.appsmith.server.repositories.UserRepository; import com.appsmith.server.repositories.WorkspaceRepository; @@ -100,8 +99,9 @@ public class AuthenticationSuccessHandlerCE implements ServerAuthenticationSucce // verification this can be eliminated safely if (user.getPassword() != null) { user.setPassword(null); - user.setSource(LoginSource.fromString( - ((OAuth2AuthenticationToken) authentication).getAuthorizedClientRegistrationId())); + user.setSource( + LoginSource.fromString(((OAuth2AuthenticationToken) authentication).getAuthorizedClientRegistrationId()) + ); // Update the user in separate thread userRepository .save(user) @@ -166,10 +166,6 @@ public class AuthenticationSuccessHandlerCE implements ServerAuthenticationSucce if (authentication instanceof OAuth2AuthenticationToken) { modeOfLogin = ((OAuth2AuthenticationToken) authentication).getAuthorizedClientRegistrationId(); } - /* - Adding default traits to flagsmith for the logged-in user - */ - monos.add(addDefaultUserTraits(user)); if (isFromSignupFinal) { final String inviteToken = currentUser.getInviteToken(); @@ -206,33 +202,6 @@ public class AuthenticationSuccessHandlerCE implements ServerAuthenticationSucce .then(redirectionMono); } - private Mono addDefaultUserTraits(User user) { - String identifier = userIdentifierService.getUserIdentifier(user); - List featureFlagTraits = new ArrayList<>(); - String emailTrait; - if (!commonConfig.isCloudHosting()) { - emailTrait = userIdentifierService.hash(user.getEmail()); - } else { - emailTrait = user.getEmail(); - } - return configService.getInstanceId().flatMap(instanceId -> { - featureFlagTraits.add(addTraitKeyValueToTraitObject(identifier, "email", emailTrait)); - featureFlagTraits.add(addTraitKeyValueToTraitObject(identifier, "instanceId", instanceId)); - featureFlagTraits.add(addTraitKeyValueToTraitObject(identifier, "tenantId", user.getTenantId())); - featureFlagTraits.add(addTraitKeyValueToTraitObject( - identifier, "is_telemetry_on", String.valueOf(!commonConfig.isTelemetryDisabled()))); - return featureFlagService.remoteSetUserTraits(featureFlagTraits); - }); - } - - private FeatureFlagTrait addTraitKeyValueToTraitObject(String identifier, String traitKey, String traitValue) { - FeatureFlagTrait featureFlagTrait = new FeatureFlagTrait(); - featureFlagTrait.setIdentifier(identifier); - featureFlagTrait.setTraitKey(traitKey); - featureFlagTrait.setTraitValue(traitValue); - return featureFlagTrait; - } - protected Mono createDefaultApplication(String defaultWorkspaceId, Authentication authentication) { // need to create default application diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/featureflags/FeatureFlagIdentities.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/featureflags/FeatureFlagIdentities.java index a720d20cb6..e69de29bb2 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/featureflags/FeatureFlagIdentities.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/featureflags/FeatureFlagIdentities.java @@ -1,16 +0,0 @@ -package com.appsmith.server.featureflags; - -import lombok.AllArgsConstructor; -import lombok.Data; -import lombok.NoArgsConstructor; - -import java.util.Set; - -@Data -@NoArgsConstructor -@AllArgsConstructor -public class FeatureFlagIdentities { - String instanceId; - String tenantId; - Set userIdentifiers; -} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/featureflags/FeatureFlagIdentityTraits.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/featureflags/FeatureFlagIdentityTraits.java new file mode 100644 index 0000000000..4e266b4d0f --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/featureflags/FeatureFlagIdentityTraits.java @@ -0,0 +1,24 @@ +package com.appsmith.server.featureflags; + +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; + +import java.util.Map; +import java.util.Set; + +/** + * FeatureFlagIdentityTraits object is used to set the default traits for a user and return the list of flags + * For older versions of self-hosted code, the `traits` was not present and only list of flags were returned + * The functionality to set default traits was added later, hence for older instances, traits remains null + * in the requests to cloud services + */ +@Data +@NoArgsConstructor +@AllArgsConstructor +public class FeatureFlagIdentityTraits { + String instanceId; + String tenantId; + Set userIdentifiers; + Map traits; +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/featureflags/FeatureFlagTrait.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/featureflags/FeatureFlagTrait.java index ef01ce1624..e69de29bb2 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/featureflags/FeatureFlagTrait.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/featureflags/FeatureFlagTrait.java @@ -1,14 +0,0 @@ -package com.appsmith.server.featureflags; - -import lombok.AllArgsConstructor; -import lombok.Data; -import lombok.NoArgsConstructor; - -@Data -@NoArgsConstructor -@AllArgsConstructor -public class FeatureFlagTrait { - String identifier; - String traitKey; - String traitValue; -} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/CacheableFeatureFlagHelperImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/CacheableFeatureFlagHelperImpl.java index b1e2b42028..cab8ddcab6 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/CacheableFeatureFlagHelperImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/CacheableFeatureFlagHelperImpl.java @@ -1,16 +1,17 @@ package com.appsmith.server.services; import com.appsmith.server.configurations.CloudServicesConfig; +import com.appsmith.server.configurations.CommonConfig; import com.appsmith.server.services.ce.CacheableFeatureFlagHelperCEImpl; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Component; @Component @Slf4j -public class CacheableFeatureFlagHelperImpl extends CacheableFeatureFlagHelperCEImpl - implements CacheableFeatureFlagHelper { - public CacheableFeatureFlagHelperImpl( - TenantService tenantService, ConfigService configService, CloudServicesConfig cloudServicesConfig) { - super(tenantService, configService, cloudServicesConfig); +public class CacheableFeatureFlagHelperImpl extends CacheableFeatureFlagHelperCEImpl implements CacheableFeatureFlagHelper { + public CacheableFeatureFlagHelperImpl(TenantService tenantService, ConfigService configService, + CloudServicesConfig cloudServicesConfig, CommonConfig commonConfig, + UserIdentifierService userIdentifierService) { + super(tenantService, configService, cloudServicesConfig, commonConfig, userIdentifierService); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/CacheableFeatureFlagHelperCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/CacheableFeatureFlagHelperCE.java index 05ded0eae8..c6015953a6 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/CacheableFeatureFlagHelperCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/CacheableFeatureFlagHelperCE.java @@ -1,11 +1,12 @@ package com.appsmith.server.services.ce; +import com.appsmith.server.domains.User; import com.appsmith.server.featureflags.CachedFlags; import reactor.core.publisher.Mono; public interface CacheableFeatureFlagHelperCE { - Mono fetchUserCachedFlags(String userIdentifier); + Mono fetchUserCachedFlags(String userIdentifier, User user); Mono evictUserCachedFlags(String userIdentifier); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/CacheableFeatureFlagHelperCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/CacheableFeatureFlagHelperCEImpl.java index 0ebbbcae9a..cdcf2e3fa6 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/CacheableFeatureFlagHelperCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/CacheableFeatureFlagHelperCEImpl.java @@ -3,13 +3,16 @@ package com.appsmith.server.services.ce; import com.appsmith.caching.annotations.Cache; import com.appsmith.caching.annotations.CacheEvict; import com.appsmith.server.configurations.CloudServicesConfig; +import com.appsmith.server.configurations.CommonConfig; +import com.appsmith.server.domains.User; import com.appsmith.server.dtos.ResponseDTO; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.featureflags.CachedFlags; -import com.appsmith.server.featureflags.FeatureFlagIdentities; +import com.appsmith.server.featureflags.FeatureFlagIdentityTraits; import com.appsmith.server.services.ConfigService; import com.appsmith.server.services.TenantService; +import com.appsmith.server.services.UserIdentifierService; import com.appsmith.util.WebClientUtils; import lombok.extern.slf4j.Slf4j; import org.springframework.core.ParameterizedTypeReference; @@ -17,6 +20,7 @@ import org.springframework.web.reactive.function.BodyInserters; import reactor.core.publisher.Mono; import java.time.Instant; +import java.util.HashMap; import java.util.Map; import java.util.Set; @@ -28,17 +32,24 @@ public class CacheableFeatureFlagHelperCEImpl implements CacheableFeatureFlagHel private final CloudServicesConfig cloudServicesConfig; - public CacheableFeatureFlagHelperCEImpl( - TenantService tenantService, ConfigService configService, CloudServicesConfig cloudServicesConfig) { + private final CommonConfig commonConfig; + + private final UserIdentifierService userIdentifierService; + + public CacheableFeatureFlagHelperCEImpl(TenantService tenantService, ConfigService configService, + CloudServicesConfig cloudServicesConfig, CommonConfig commonConfig, + UserIdentifierService userIdentifierService) { this.tenantService = tenantService; this.configService = configService; this.cloudServicesConfig = cloudServicesConfig; + this.commonConfig = commonConfig; + this.userIdentifierService = userIdentifierService; } @Cache(cacheName = "featureFlag", key = "{#userIdentifier}") @Override - public Mono fetchUserCachedFlags(String userIdentifier) { - return this.forceAllRemoteFeatureFlagsForUser(userIdentifier).flatMap(flags -> { + public Mono fetchUserCachedFlags(String userIdentifier, User user) { + return this.forceAllRemoteFeatureFlagsForUser(userIdentifier, user).flatMap(flags -> { CachedFlags cachedFlags = new CachedFlags(); cachedFlags.setRefreshedAt(Instant.now()); cachedFlags.setFlags(flags); @@ -46,33 +57,67 @@ public class CacheableFeatureFlagHelperCEImpl implements CacheableFeatureFlagHel }); } + private Mono> getUserDefaultTraits(User user) { + return configService.getInstanceId() + .map(instanceId -> { + Map userTraits = new HashMap<>(); + String emailTrait; + if (!commonConfig.isCloudHosting()) { + emailTrait = userIdentifierService.hash(user.getEmail()); + } else { + emailTrait = user.getEmail(); + } + userTraits.put("email", emailTrait); + userTraits.put("instanceId", instanceId); + userTraits.put("tenantId", user.getTenantId()); + userTraits.put("isTelemetryOn", !commonConfig.isTelemetryDisabled()); + userTraits.put("createdAt", user.getCreatedAt()); + userTraits.put("defaultTraitsUpdatedAt", Instant.now().getEpochSecond()); + userTraits.put("type", "user"); + return userTraits; + }); + } + @CacheEvict(cacheName = "featureFlag", key = "{#userIdentifier}") @Override public Mono evictUserCachedFlags(String userIdentifier) { return Mono.empty(); } - private Mono> forceAllRemoteFeatureFlagsForUser(String userIdentifier) { + private Mono> forceAllRemoteFeatureFlagsForUser(String userIdentifier, User user) { Mono instanceIdMono = configService.getInstanceId(); // TODO: Convert to current tenant when the feature is enabled Mono defaultTenantIdMono = tenantService.getDefaultTenantId(); - return Mono.zip(instanceIdMono, defaultTenantIdMono) - .flatMap(tuple2 -> { + return Mono.zip(instanceIdMono, defaultTenantIdMono, getUserDefaultTraits(user)) + .flatMap(objects -> { return this.getRemoteFeatureFlagsByIdentity( - new FeatureFlagIdentities(tuple2.getT1(), tuple2.getT2(), Set.of(userIdentifier))); + new FeatureFlagIdentityTraits( + objects.getT1(), + objects.getT2(), + Set.of(userIdentifier), + objects.getT3()) + ); }) .map(newValue -> newValue.get(userIdentifier)); } - private Mono>> getRemoteFeatureFlagsByIdentity(FeatureFlagIdentities identity) { + /** + * This method will call the cloud services which will call the flagsmith sdk. + * The default traits and the user identifier are passed to flagsmith sdk which internally will set the traits + * for the user and also returns the flags in the same sdk call. + * @param featureFlagIdentityTraits + * @return + */ + private Mono>> getRemoteFeatureFlagsByIdentity(FeatureFlagIdentityTraits featureFlagIdentityTraits) { return WebClientUtils.create(cloudServicesConfig.getBaseUrl()) .post() .uri("/api/v1/feature-flags") - .body(BodyInserters.fromValue(identity)) + .body(BodyInserters.fromValue(featureFlagIdentityTraits)) .exchangeToMono(clientResponse -> { if (clientResponse.statusCode().is2xxSuccessful()) { - return clientResponse.bodyToMono( - new ParameterizedTypeReference>>>() {}); + return clientResponse.bodyToMono(new ParameterizedTypeReference>>>() { + }); } else { return clientResponse.createError(); } @@ -81,7 +126,8 @@ public class CacheableFeatureFlagHelperCEImpl implements CacheableFeatureFlagHel .onErrorMap( // Only map errors if we haven't already wrapped them into an AppsmithException e -> !(e instanceof AppsmithException), - e -> new AppsmithException(AppsmithError.CLOUD_SERVICES_ERROR, e.getMessage())) + e -> new AppsmithException(AppsmithError.CLOUD_SERVICES_ERROR, e.getMessage()) + ) .onErrorResume(error -> { // We're gobbling up errors here so that all feature flags are turned off by default // This will be problematic if we do not maintain code to reflect validity of flags diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/FeatureFlagServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/FeatureFlagServiceCE.java index 771584a80c..4f5ae85ddb 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/FeatureFlagServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/FeatureFlagServiceCE.java @@ -2,7 +2,6 @@ package com.appsmith.server.services.ce; import com.appsmith.server.domains.User; import com.appsmith.server.featureflags.FeatureFlagEnum; -import com.appsmith.server.featureflags.FeatureFlagTrait; import reactor.core.publisher.Mono; import java.util.List; @@ -34,9 +33,8 @@ public interface FeatureFlagServiceCE { /** * Fetch all the flags and their values for the current logged in user * - * @return Mono> + * @return Mono> */ Mono> getAllFeatureFlagsForUser(); - Mono remoteSetUserTraits(List featureFlagTraits); -} +} \ No newline at end of file diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/FeatureFlagServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/FeatureFlagServiceCEImpl.java index a6c75a78ad..6db4d63376 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/FeatureFlagServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/FeatureFlagServiceCEImpl.java @@ -3,32 +3,25 @@ package com.appsmith.server.services.ce; import com.appsmith.server.configurations.CloudServicesConfig; import com.appsmith.server.constants.FieldName; import com.appsmith.server.domains.User; -import com.appsmith.server.dtos.ResponseDTO; -import com.appsmith.server.exceptions.AppsmithError; -import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.featureflags.FeatureFlagEnum; -import com.appsmith.server.featureflags.FeatureFlagTrait; import com.appsmith.server.services.CacheableFeatureFlagHelper; import com.appsmith.server.services.ConfigService; import com.appsmith.server.services.SessionUserService; import com.appsmith.server.services.TenantService; import com.appsmith.server.services.UserIdentifierService; -import com.appsmith.util.WebClientUtils; import lombok.extern.slf4j.Slf4j; import org.ff4j.FF4j; import org.ff4j.core.FlippingExecutionContext; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.core.ParameterizedTypeReference; -import org.springframework.web.reactive.function.BodyInserters; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.util.function.Tuple2; import java.time.Instant; import java.time.temporal.ChronoUnit; -import java.util.List; import java.util.Map; + @Slf4j public class FeatureFlagServiceCEImpl implements FeatureFlagServiceCE { @@ -49,14 +42,13 @@ public class FeatureFlagServiceCEImpl implements FeatureFlagServiceCE { private final CacheableFeatureFlagHelper cacheableFeatureFlagHelper; @Autowired - public FeatureFlagServiceCEImpl( - SessionUserService sessionUserService, - FF4j ff4j, - TenantService tenantService, - ConfigService configService, - CloudServicesConfig cloudServicesConfig, - UserIdentifierService userIdentifierService, - CacheableFeatureFlagHelper cacheableFeatureFlagHelper) { + public FeatureFlagServiceCEImpl(SessionUserService sessionUserService, + FF4j ff4j, + TenantService tenantService, + ConfigService configService, + CloudServicesConfig cloudServicesConfig, + UserIdentifierService userIdentifierService, + CacheableFeatureFlagHelper cacheableFeatureFlagHelper) { this.sessionUserService = sessionUserService; this.ff4j = ff4j; this.tenantService = tenantService; @@ -66,6 +58,7 @@ public class FeatureFlagServiceCEImpl implements FeatureFlagServiceCE { this.cacheableFeatureFlagHelper = cacheableFeatureFlagHelper; } + private Mono checkAll(String featureName, User user) { Boolean check = check(featureName, user); @@ -88,7 +81,8 @@ public class FeatureFlagServiceCEImpl implements FeatureFlagServiceCE { @Override public Mono check(FeatureFlagEnum featureEnum) { - return sessionUserService.getCurrentUser().flatMap(user -> check(featureEnum, user)); + return sessionUserService.getCurrentUser() + .flatMap(user -> check(featureEnum, user)); } @Override @@ -99,13 +93,15 @@ public class FeatureFlagServiceCEImpl implements FeatureFlagServiceCE { @Override public Mono> getAllFeatureFlagsForUser() { Mono currentUser = sessionUserService.getCurrentUser().cache(); - Flux> featureUserTuple = Flux.fromIterable( - ff4j.getFeatures().keySet()) + Flux> featureUserTuple = Flux.fromIterable(ff4j.getFeatures().keySet()) .flatMap(featureName -> Mono.just(featureName).zipWith(currentUser)); Mono> localFlagsForUser = featureUserTuple .filter(objects -> !objects.getT2().isAnonymous()) - .collectMap(Tuple2::getT1, tuple -> check(tuple.getT1(), tuple.getT2())); + .collectMap( + Tuple2::getT1, + tuple -> check(tuple.getT1(), tuple.getT2()) + ); return Mono.zip(localFlagsForUser, this.getAllRemoteFeatureFlagsForUser()) .map(tuple -> { @@ -121,50 +117,22 @@ public class FeatureFlagServiceCEImpl implements FeatureFlagServiceCE { */ private Mono> getAllRemoteFeatureFlagsForUser() { Mono userMono = sessionUserService.getCurrentUser().cache(); - return userMono.flatMap(user -> { - String userIdentifier = userIdentifierService.getUserIdentifier(user); - // Checks for flags present in cache and if the cache is not expired - return cacheableFeatureFlagHelper - .fetchUserCachedFlags(userIdentifier) - .flatMap(cachedFlags -> { - if (cachedFlags.getRefreshedAt().until(Instant.now(), ChronoUnit.MINUTES) - < this.featureFlagCacheTimeMin) { - return Mono.just(cachedFlags.getFlags()); - } else { - // empty the cache for the userIdentifier as expired - return cacheableFeatureFlagHelper - .evictUserCachedFlags(userIdentifier) - .then(cacheableFeatureFlagHelper.fetchUserCachedFlags(userIdentifier)) - .flatMap(cachedFlagsUpdated -> Mono.just(cachedFlagsUpdated.getFlags())); - } - }); - }); - } - - @Override - public Mono remoteSetUserTraits(List featureFlagTraits) { - - return WebClientUtils.create(cloudServicesConfig.getBaseUrl()) - .post() - .uri("/api/v1/feature-flags/trait") - .body(BodyInserters.fromValue(featureFlagTraits)) - .exchangeToMono(clientResponse -> { - if (clientResponse.statusCode().is2xxSuccessful()) { - return clientResponse.bodyToMono(new ParameterizedTypeReference>() {}); - } else { - return clientResponse.createError(); - } - }) - .map(ResponseDTO::getData) - .onErrorMap( - // Only map errors if we haven't already wrapped them into an AppsmithException - e -> !(e instanceof AppsmithException), - e -> new AppsmithException(AppsmithError.CLOUD_SERVICES_ERROR, e.getMessage())) - .onErrorResume(error -> { - // We're gobbling up errors here so that all feature flags are turned off by default - // This will be problematic if we do not maintain code to reflect validity of flags - log.debug("Received error from CS for feature flags: {}", error.getMessage()); - return Mono.empty(); + return userMono + .flatMap(user -> { + String userIdentifier = userIdentifierService.getUserIdentifier(user); + // Checks for flags present in cache and if the cache is not expired + return cacheableFeatureFlagHelper + .fetchUserCachedFlags(userIdentifier, user) + .flatMap(cachedFlags -> { + if (cachedFlags.getRefreshedAt().until(Instant.now(), ChronoUnit.MINUTES) < this.featureFlagCacheTimeMin) { + return Mono.just(cachedFlags.getFlags()); + } else { + // empty the cache for the userIdentifier as expired + return cacheableFeatureFlagHelper.evictUserCachedFlags(userIdentifier) + .then(cacheableFeatureFlagHelper.fetchUserCachedFlags(userIdentifier, user)) + .flatMap(cachedFlagsUpdated -> Mono.just(cachedFlagsUpdated.getFlags())); + } + }); }); } -} +} \ No newline at end of file diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/FeatureFlagServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/FeatureFlagServiceTest.java index f0a0e2e5d7..bf6a71454a 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/FeatureFlagServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/FeatureFlagServiceTest.java @@ -1,5 +1,6 @@ package com.appsmith.server.services; +import com.appsmith.server.domains.User; import com.appsmith.server.featureflags.CachedFlags; import com.appsmith.server.featureflags.FeatureFlagEnum; import lombok.extern.slf4j.Slf4j; @@ -22,6 +23,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; + @ExtendWith(SpringExtension.class) @SpringBootTest @Slf4j @@ -90,9 +92,10 @@ public class FeatureFlagServiceTest { } @Test - public void getFeatureFlags_withUserIdentifier_redisKeyExists() { + public void getFeatureFlags_withUserIdentifier_redisKeyExists(){ String userIdentifier = "testIdentifier"; - Mono cachedFlagsMono = cacheableFeatureFlagHelper.fetchUserCachedFlags(userIdentifier); + User dummyUser = new User(); + Mono cachedFlagsMono = cacheableFeatureFlagHelper.fetchUserCachedFlags(userIdentifier, dummyUser); Mono hasKeyMono = reactiveRedisTemplate.hasKey("featureFlag:" + userIdentifier); StepVerifier.create(cachedFlagsMono.then(hasKeyMono)) .assertNext(isKeyPresent -> { @@ -124,4 +127,5 @@ public class FeatureFlagServiceTest { return ff4j; } } + }