feat: Feature Flagging Default Traits (#25201)
## Description > This PR sets the default traits for the users in the same SDK call to get features. > This ensures the traits are present for all users and also the SDK calls are contained. Fixes #25159 #### Media > A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video > > #### Type of change - New feature (non-breaking change which adds functionality) ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [x] Manual - [x] Jest - [ ] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed
This commit is contained in:
parent
2fc20cfe8e
commit
5dcc1352c0
15
CODEOWNERS
15
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
|
||||
|
|
|
|||
|
|
@ -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<Void> addDefaultUserTraits(User user) {
|
||||
String identifier = userIdentifierService.getUserIdentifier(user);
|
||||
List<FeatureFlagTrait> 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<Application> createDefaultApplication(String defaultWorkspaceId, Authentication authentication) {
|
||||
|
||||
// need to create default application
|
||||
|
|
|
|||
|
|
@ -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<String> userIdentifiers;
|
||||
}
|
||||
|
|
@ -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<String> userIdentifiers;
|
||||
Map<String, Object> traits;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<CachedFlags> fetchUserCachedFlags(String userIdentifier);
|
||||
Mono<CachedFlags> fetchUserCachedFlags(String userIdentifier, User user);
|
||||
|
||||
Mono<Void> evictUserCachedFlags(String userIdentifier);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<CachedFlags> fetchUserCachedFlags(String userIdentifier) {
|
||||
return this.forceAllRemoteFeatureFlagsForUser(userIdentifier).flatMap(flags -> {
|
||||
public Mono<CachedFlags> 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<Map<String, Object>> getUserDefaultTraits(User user) {
|
||||
return configService.getInstanceId()
|
||||
.map(instanceId -> {
|
||||
Map<String, Object> 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<Void> evictUserCachedFlags(String userIdentifier) {
|
||||
return Mono.empty();
|
||||
}
|
||||
|
||||
private Mono<Map<String, Boolean>> forceAllRemoteFeatureFlagsForUser(String userIdentifier) {
|
||||
private Mono<Map<String, Boolean>> forceAllRemoteFeatureFlagsForUser(String userIdentifier, User user) {
|
||||
Mono<String> instanceIdMono = configService.getInstanceId();
|
||||
// TODO: Convert to current tenant when the feature is enabled
|
||||
Mono<String> 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<Map<String, Map<String, Boolean>>> 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<Map<String, Map<String, Boolean>>> 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<ResponseDTO<Map<String, Map<String, Boolean>>>>() {});
|
||||
return clientResponse.bodyToMono(new ParameterizedTypeReference<ResponseDTO<Map<String,
|
||||
Map<String, Boolean>>>>() {
|
||||
});
|
||||
} 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
|
||||
|
|
|
|||
|
|
@ -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<Map<String, Boolean>>
|
||||
* @return Mono<Map < String, Boolean>>
|
||||
*/
|
||||
Mono<Map<String, Boolean>> getAllFeatureFlagsForUser();
|
||||
|
||||
Mono<Void> remoteSetUserTraits(List<FeatureFlagTrait> featureFlagTraits);
|
||||
}
|
||||
}
|
||||
|
|
@ -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<Boolean> checkAll(String featureName, User user) {
|
||||
Boolean check = check(featureName, user);
|
||||
|
||||
|
|
@ -88,7 +81,8 @@ public class FeatureFlagServiceCEImpl implements FeatureFlagServiceCE {
|
|||
|
||||
@Override
|
||||
public Mono<Boolean> 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<Map<String, Boolean>> getAllFeatureFlagsForUser() {
|
||||
Mono<User> currentUser = sessionUserService.getCurrentUser().cache();
|
||||
Flux<Tuple2<String, User>> featureUserTuple = Flux.fromIterable(
|
||||
ff4j.getFeatures().keySet())
|
||||
Flux<Tuple2<String, User>> featureUserTuple = Flux.fromIterable(ff4j.getFeatures().keySet())
|
||||
.flatMap(featureName -> Mono.just(featureName).zipWith(currentUser));
|
||||
|
||||
Mono<Map<String, Boolean>> 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<Map<String, Boolean>> getAllRemoteFeatureFlagsForUser() {
|
||||
Mono<User> 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<Void> remoteSetUserTraits(List<FeatureFlagTrait> 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<ResponseDTO<Void>>() {});
|
||||
} 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()));
|
||||
}
|
||||
});
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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<CachedFlags> cachedFlagsMono = cacheableFeatureFlagHelper.fetchUserCachedFlags(userIdentifier);
|
||||
User dummyUser = new User();
|
||||
Mono<CachedFlags> cachedFlagsMono = cacheableFeatureFlagHelper.fetchUserCachedFlags(userIdentifier, dummyUser);
|
||||
Mono<Boolean> hasKeyMono = reactiveRedisTemplate.hasKey("featureFlag:" + userIdentifier);
|
||||
StepVerifier.create(cachedFlagsMono.then(hasKeyMono))
|
||||
.assertNext(isKeyPresent -> {
|
||||
|
|
@ -124,4 +127,5 @@ public class FeatureFlagServiceTest {
|
|||
return ff4j;
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user