chore: remove FF4J (#28653)
## Description > Remove FF4J #### PR fixes following issue(s) Fixes #24872 > if no issue exists, please create an issue and ask the maintainers about this first > > #### 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 - Chore (housekeeping or task changes that don't impact user perception) ## Testing > #### How Has This Been Tested? - [ ] Manual - [x] JUnit - [ ] 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 - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] 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 is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Simplified the feature flag checking process, removing dependencies on external libraries. - Improved error handling in feature flag services for more robust operation. - **Bug Fixes** - Adjusted feature flag refresh logic to ensure accurate status representation. - **Tests** - Updated test cases to align with the new feature flag checking logic and error handling improvements. - **Documentation** - Removed outdated comments and documentation related to the old feature flag system. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Nilesh Sarupriya <20905988+nsarupr@users.noreply.github.com> Co-authored-by: Abhijeet <abhi.nagarnaik@gmail.com>
This commit is contained in:
Nilesh Sarupriya
GitHub
parent
9632bca064
commit
165856e885
|
|
@ -1,17 +0,0 @@
|
|||
package com.appsmith.server.configurations;
|
||||
|
||||
import org.ff4j.FF4j;
|
||||
import org.ff4j.conf.XmlParser;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
|
||||
@Configuration
|
||||
public class FeatureFlagConfig {
|
||||
|
||||
// Since we rely on cloud services to retrieve the most up-to-date flags, we avoid automatically generating the same
|
||||
// flag in the FF4J context.
|
||||
@Bean
|
||||
public FF4j ff4j() {
|
||||
return new FF4j(new XmlParser(), "features/init-flags.xml").audit(true).autoCreate(false);
|
||||
}
|
||||
}
|
||||
|
|
@ -1,19 +1,5 @@
|
|||
package com.appsmith.server.featureflags;
|
||||
|
||||
import org.ff4j.core.FlippingStrategy;
|
||||
import org.ff4j.strategy.PonderationStrategy;
|
||||
import org.ff4j.strategy.time.OfficeHourStrategy;
|
||||
|
||||
/**
|
||||
* This enum lists all the feature flags available along with their flipping strategy.
|
||||
* In order to create a new feature flag, create another enum entry and add the same string to {@link features/init-flags.xml}
|
||||
* <p>
|
||||
* If you wish to define a custom flipping strategy, define a class that implements {@link FlippingStrategy} and
|
||||
* ensure that you've mentioned this custom class when defining the feature in {@link features/init-flags.xml}
|
||||
* <p>
|
||||
* The feature flag implementation class should extend an existing feature flag implementation like {@link PonderationStrategy},
|
||||
* {@link OfficeHourStrategy} etc. These default classes provide a lot of basic functionality out of the box.
|
||||
*/
|
||||
public enum FeatureFlagEnum {
|
||||
// ------------------- These features are only for JUnit testing. DO NOT use these features in your code path.--- //
|
||||
// ------------------- Couldn't find a better way to do this ---------------------------------------------------- //
|
||||
|
|
|
|||
|
|
@ -1,24 +0,0 @@
|
|||
package com.appsmith.server.featureflags.strategies;
|
||||
|
||||
import com.appsmith.server.constants.FieldName;
|
||||
import com.appsmith.server.domains.User;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.apache.commons.lang3.StringUtils;
|
||||
import org.ff4j.core.FeatureStore;
|
||||
import org.ff4j.core.FlippingExecutionContext;
|
||||
import org.ff4j.strategy.AbstractFlipStrategy;
|
||||
|
||||
/**
|
||||
* This strategy enables a given feature for Appsmith users only. Useful when features are under development and not
|
||||
* ready for prime-time
|
||||
*/
|
||||
@Slf4j
|
||||
public class AppsmithUserStrategy extends AbstractFlipStrategy {
|
||||
|
||||
@Override
|
||||
public boolean evaluate(String featureName, FeatureStore store, FlippingExecutionContext executionContext) {
|
||||
User user = (User) executionContext.getValue(FieldName.USER, true);
|
||||
|
||||
return StringUtils.endsWith(user.getEmail(), "@appsmith.com");
|
||||
}
|
||||
}
|
||||
|
|
@ -1,63 +0,0 @@
|
|||
package com.appsmith.server.featureflags.strategies;
|
||||
|
||||
import com.appsmith.server.constants.FieldName;
|
||||
import com.appsmith.server.domains.User;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.apache.commons.lang3.StringUtils;
|
||||
import org.ff4j.core.FeatureStore;
|
||||
import org.ff4j.core.FlippingExecutionContext;
|
||||
import org.ff4j.strategy.AbstractFlipStrategy;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
@Slf4j
|
||||
public class EmailBasedRolloutStrategy extends AbstractFlipStrategy {
|
||||
|
||||
List<String> validDomains = new ArrayList<>();
|
||||
List<String> validEmails = new ArrayList<>();
|
||||
|
||||
private static final String PARAM_EMAIL_DOMAINS = "emailDomains";
|
||||
private static final String PARAM_EMAILS = "emails";
|
||||
|
||||
/**
|
||||
* {@inheritDoc}
|
||||
*/
|
||||
@Override
|
||||
public void init(String featureName, Map<String, String> initParam) {
|
||||
super.init(featureName, initParam);
|
||||
if (!initParam.containsKey(PARAM_EMAIL_DOMAINS) && !initParam.containsKey(PARAM_EMAILS)) {
|
||||
String msg = String.format(
|
||||
"Either '%s' or '%s' is required for EmailBasedRolloutStrategy", PARAM_EMAIL_DOMAINS, PARAM_EMAILS);
|
||||
throw new IllegalArgumentException(msg);
|
||||
}
|
||||
if (!StringUtils.isEmpty(initParam.get(PARAM_EMAIL_DOMAINS))) {
|
||||
this.validDomains = Arrays.asList(initParam.get(PARAM_EMAIL_DOMAINS).split(","));
|
||||
}
|
||||
if (!StringUtils.isEmpty(initParam.get(PARAM_EMAILS))) {
|
||||
this.validEmails = Arrays.asList(initParam.get(PARAM_EMAILS).split(","));
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* {@inheritDoc}
|
||||
*/
|
||||
@Override
|
||||
public boolean evaluate(String featureName, FeatureStore store, FlippingExecutionContext executionContext) {
|
||||
User user = (User) executionContext.getValue(FieldName.USER, true);
|
||||
int atIndex = user.getEmail().indexOf("@");
|
||||
|
||||
if (atIndex > 0) {
|
||||
// If the email domain is valid, check the user's email ID against the list of validated domains
|
||||
String domain = user.getEmail().substring(atIndex + 1).toLowerCase();
|
||||
if (validDomains.contains(domain)) {
|
||||
return true;
|
||||
} else {
|
||||
return validEmails.contains(user.getEmail());
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
|
@ -2,21 +2,18 @@ package com.appsmith.server.services;
|
|||
|
||||
import com.appsmith.server.helpers.FeatureFlagMigrationHelper;
|
||||
import com.appsmith.server.services.ce.FeatureFlagServiceCEImpl;
|
||||
import org.ff4j.FF4j;
|
||||
import org.springframework.stereotype.Component;
|
||||
|
||||
@Component
|
||||
public class FeatureFlagServiceImpl extends FeatureFlagServiceCEImpl implements FeatureFlagService {
|
||||
public FeatureFlagServiceImpl(
|
||||
SessionUserService sessionUserService,
|
||||
FF4j ff4j,
|
||||
TenantService tenantService,
|
||||
UserIdentifierService userIdentifierService,
|
||||
CacheableFeatureFlagHelper cacheableFeatureFlagHelper,
|
||||
FeatureFlagMigrationHelper featureFlagMigrationHelper) {
|
||||
super(
|
||||
sessionUserService,
|
||||
ff4j,
|
||||
tenantService,
|
||||
userIdentifierService,
|
||||
cacheableFeatureFlagHelper,
|
||||
|
|
|
|||
|
|
@ -152,7 +152,7 @@ public class CacheableFeatureFlagHelperCEImpl implements CacheableFeatureFlagHel
|
|||
// 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.just(Map.of());
|
||||
return Mono.just(new HashMap<>());
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -166,8 +166,14 @@ public class CacheableFeatureFlagHelperCEImpl implements CacheableFeatureFlagHel
|
|||
public Mono<CachedFeatures> fetchCachedTenantFeatures(String tenantId) {
|
||||
return this.forceAllRemoteFeaturesForTenant(tenantId).flatMap(flags -> {
|
||||
CachedFeatures cachedFeatures = new CachedFeatures();
|
||||
cachedFeatures.setRefreshedAt(Instant.now());
|
||||
cachedFeatures.setFeatures(flags);
|
||||
// If CS is down we expect the empty flags, from upstream method. Hence, setting the refreshed at to past
|
||||
// so that the next call will have the force refresh.
|
||||
if (CollectionUtils.isNullOrEmpty(flags)) {
|
||||
cachedFeatures.setRefreshedAt(Instant.now().minus(1, ChronoUnit.DAYS));
|
||||
} else {
|
||||
cachedFeatures.setRefreshedAt(Instant.now());
|
||||
}
|
||||
return Mono.just(cachedFeatures);
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,7 +1,6 @@
|
|||
package com.appsmith.server.services.ce;
|
||||
|
||||
import com.appsmith.server.domains.Tenant;
|
||||
import com.appsmith.server.domains.User;
|
||||
import com.appsmith.server.featureflags.CachedFeatures;
|
||||
import com.appsmith.server.featureflags.FeatureFlagEnum;
|
||||
import reactor.core.publisher.Mono;
|
||||
|
|
@ -10,16 +9,6 @@ import java.util.Map;
|
|||
|
||||
public interface FeatureFlagServiceCE {
|
||||
|
||||
/**
|
||||
* Used to check if a particular feature is enabled for a given user. Useful in contexts where we already have the
|
||||
* User object and simply wish to do a boolean check
|
||||
*
|
||||
* @param featureEnum
|
||||
* @param user
|
||||
* @return Boolean
|
||||
*/
|
||||
Mono<Boolean> check(FeatureFlagEnum featureEnum, User user);
|
||||
|
||||
/**
|
||||
* Check if a particular feature is enabled for the current logged in user. Useful in chaining reactive functions
|
||||
* while writing business logic that may depend on a feature flag
|
||||
|
|
@ -29,8 +18,6 @@ public interface FeatureFlagServiceCE {
|
|||
*/
|
||||
Mono<Boolean> check(FeatureFlagEnum featureEnum);
|
||||
|
||||
Boolean check(String featureName, User user);
|
||||
|
||||
/**
|
||||
* Fetch all the flags and their values for the current logged in user
|
||||
*
|
||||
|
|
|
|||
|
|
@ -1,6 +1,5 @@
|
|||
package com.appsmith.server.services.ce;
|
||||
|
||||
import com.appsmith.server.constants.FieldName;
|
||||
import com.appsmith.server.constants.MigrationStatus;
|
||||
import com.appsmith.server.domains.Tenant;
|
||||
import com.appsmith.server.domains.TenantConfiguration;
|
||||
|
|
@ -16,19 +15,13 @@ import com.appsmith.server.services.TenantService;
|
|||
import com.appsmith.server.services.UserIdentifierService;
|
||||
import lombok.RequiredArgsConstructor;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.ff4j.FF4j;
|
||||
import org.ff4j.core.FlippingExecutionContext;
|
||||
import org.ff4j.exception.FeatureNotFoundException;
|
||||
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.HashMap;
|
||||
import java.util.Map;
|
||||
|
||||
import static java.lang.Boolean.TRUE;
|
||||
import java.util.Objects;
|
||||
|
||||
@Slf4j
|
||||
@RequiredArgsConstructor
|
||||
|
|
@ -36,8 +29,6 @@ public class FeatureFlagServiceCEImpl implements FeatureFlagServiceCE {
|
|||
|
||||
private final SessionUserService sessionUserService;
|
||||
|
||||
private final FF4j ff4j;
|
||||
|
||||
private final TenantService tenantService;
|
||||
|
||||
private final UserIdentifierService userIdentifierService;
|
||||
|
|
@ -49,26 +40,6 @@ public class FeatureFlagServiceCEImpl implements FeatureFlagServiceCE {
|
|||
|
||||
private CachedFeatures cachedTenantFeatureFlags;
|
||||
|
||||
private Mono<Boolean> checkAll(String featureName, User user) {
|
||||
Boolean check = check(featureName, user);
|
||||
|
||||
if (TRUE.equals(check)) {
|
||||
return Mono.just(true);
|
||||
}
|
||||
|
||||
return getAllFeatureFlagsForUser()
|
||||
.flatMap(featureMap -> Mono.justOrEmpty(featureMap.get(featureName)))
|
||||
.switchIfEmpty(Mono.just(false));
|
||||
}
|
||||
|
||||
@Override
|
||||
public Mono<Boolean> check(FeatureFlagEnum featureEnum, User user) {
|
||||
if (featureEnum == null) {
|
||||
return Mono.just(false);
|
||||
}
|
||||
return checkAll(featureEnum.toString(), user);
|
||||
}
|
||||
|
||||
/**
|
||||
* This function checks if the feature is enabled for the current user. In case the user object is not present,
|
||||
* i.e. when the method is getting called internally via cron or other mechanism check for tenant level flag and
|
||||
|
|
@ -79,32 +50,11 @@ public class FeatureFlagServiceCEImpl implements FeatureFlagServiceCE {
|
|||
*/
|
||||
@Override
|
||||
public Mono<Boolean> check(FeatureFlagEnum featureEnum) {
|
||||
// Check if the feature is supported at the tenant level and provide a fallback as falsy value
|
||||
// i.e. not supported
|
||||
Mono<Boolean> isTenantFeatureSupported = this.getTenantFeatures()
|
||||
.flatMap(featureMap -> Mono.justOrEmpty(featureMap.get(featureEnum.name())))
|
||||
.switchIfEmpty(Mono.just(false));
|
||||
|
||||
return sessionUserService
|
||||
.getCurrentUser()
|
||||
.flatMap(user -> check(featureEnum, user))
|
||||
.switchIfEmpty(isTenantFeatureSupported);
|
||||
}
|
||||
|
||||
@Override
|
||||
public Boolean check(String featureName, User user) {
|
||||
try {
|
||||
return ff4j.check(featureName, new FlippingExecutionContext(Map.of(FieldName.USER, user)));
|
||||
} catch (Exception e) {
|
||||
// FF4J is configured not to auto-generate a flag if it's not present in init-flags.xml
|
||||
// (see FeatureFlagConfig.java).
|
||||
// Consequently, we anticipate that the flag may not exist in the FF4J context and need to handle any
|
||||
// related exceptions silently.
|
||||
if (!(e instanceof FeatureNotFoundException)) {
|
||||
log.error("Error checking feature flag: {}", featureName, e);
|
||||
}
|
||||
if (Objects.isNull(featureEnum)) {
|
||||
return Mono.just(Boolean.FALSE);
|
||||
}
|
||||
return false;
|
||||
return this.getAllFeatureFlagsForUser()
|
||||
.map(featureMap -> featureMap.getOrDefault(featureEnum.name(), Boolean.FALSE));
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -115,27 +65,16 @@ 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())
|
||||
.flatMap(featureName -> Mono.just(featureName).zipWith(currentUser));
|
||||
|
||||
// Filter out anonymous users, then collect feature flags into a Map
|
||||
Mono<Map<String, Boolean>> localFlagsForUser = featureUserTuple
|
||||
.filter(objects -> !objects.getT2().isAnonymous())
|
||||
.collectMap(Tuple2::getT1, tuple -> check(tuple.getT1(), tuple.getT2()));
|
||||
|
||||
// Combine local flags, remote flags, and tenant features, and merge them into a single map
|
||||
return localFlagsForUser.flatMap(localFlags -> this.getAllRemoteFeatureFlagsForUser()
|
||||
.zipWith(this.getTenantFeatures())
|
||||
return Mono.zip(this.getAllRemoteFeatureFlagsForUser(), this.getTenantFeatures())
|
||||
.map(remoteAndTenantFlags -> {
|
||||
Map<String, Boolean> combinedFlags = new HashMap<>(localFlags);
|
||||
Map<String, Boolean> combinedFlags = new HashMap<>();
|
||||
combinedFlags.putAll(remoteAndTenantFlags.getT1());
|
||||
// Always add the tenant level flags after the user flags to make sure tenant flags gets the
|
||||
// precedence
|
||||
combinedFlags.putAll(remoteAndTenantFlags.getT2());
|
||||
return combinedFlags;
|
||||
}));
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -146,33 +85,33 @@ 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, user)
|
||||
.flatMap(cachedFlags -> {
|
||||
if (cachedFlags.getRefreshedAt().until(Instant.now(), ChronoUnit.MINUTES)
|
||||
< FEATURE_FLAG_CACHE_TIME_MIN) {
|
||||
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 -> {
|
||||
// In case the retrieval of the latest flags from CS encounters an error, the
|
||||
// previous flags will serve as a fallback value.
|
||||
if (cachedFlagsUpdated == null
|
||||
|| CollectionUtils.isNullOrEmpty(cachedFlagsUpdated.getFlags())) {
|
||||
return cacheableFeatureFlagHelper
|
||||
.updateUserCachedFlags(userIdentifier, cachedFlags)
|
||||
.map(CachedFlags::getFlags);
|
||||
}
|
||||
return Mono.just(cachedFlagsUpdated.getFlags());
|
||||
});
|
||||
}
|
||||
});
|
||||
});
|
||||
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)
|
||||
< FEATURE_FLAG_CACHE_TIME_MIN) {
|
||||
return Mono.just(cachedFlags.getFlags());
|
||||
}
|
||||
// empty the cache for the userIdentifier as expired
|
||||
return cacheableFeatureFlagHelper
|
||||
.evictUserCachedFlags(userIdentifier)
|
||||
.then(cacheableFeatureFlagHelper.fetchUserCachedFlags(userIdentifier, user))
|
||||
.flatMap(cachedFlagsUpdated -> {
|
||||
// In case the retrieval of the latest flags from CS encounters an error,
|
||||
// the previous flags will serve as a fallback value.
|
||||
if (cachedFlagsUpdated == null
|
||||
|| CollectionUtils.isNullOrEmpty(cachedFlagsUpdated.getFlags())) {
|
||||
return cacheableFeatureFlagHelper
|
||||
.updateUserCachedFlags(userIdentifier, cachedFlags)
|
||||
.map(CachedFlags::getFlags);
|
||||
}
|
||||
return Mono.just(cachedFlagsUpdated.getFlags());
|
||||
});
|
||||
});
|
||||
})
|
||||
.switchIfEmpty(Mono.just(new HashMap<>()));
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -223,7 +162,8 @@ public class FeatureFlagServiceCEImpl implements FeatureFlagServiceCE {
|
|||
.map(cachedFeatures -> {
|
||||
cachedTenantFeatureFlags = cachedFeatures;
|
||||
return cachedFeatures.getFeatures();
|
||||
});
|
||||
})
|
||||
.switchIfEmpty(Mono.just(new HashMap<>()));
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -1,13 +0,0 @@
|
|||
<?xml version="1.0" encoding="UTF-8" ?>
|
||||
<ff4j>
|
||||
<autocreate>false</autocreate>
|
||||
<audit>false</audit>
|
||||
<features>
|
||||
<feature uid="APP_NAVIGATION_LOGO_UPLOAD" enable="true"
|
||||
description="Logo upload feature for app viewer navigation">
|
||||
<flipstrategy class="com.appsmith.server.featureflags.strategies.EmailBasedRolloutStrategy">
|
||||
<param name="emailDomains" value="appsmith.com,moolya.com"/>
|
||||
</flipstrategy>
|
||||
</feature>
|
||||
</features>
|
||||
</ff4j>
|
||||
|
|
@ -4,6 +4,7 @@ import com.appsmith.server.aspect.component.TestComponent;
|
|||
import com.appsmith.server.featureflags.CachedFeatures;
|
||||
import com.appsmith.server.featureflags.FeatureFlagEnum;
|
||||
import com.appsmith.server.services.FeatureFlagService;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.extension.ExtendWith;
|
||||
import org.mockito.Mockito;
|
||||
|
|
@ -34,6 +35,16 @@ class FeatureFlaggedMethodInvokerAspectTest {
|
|||
private static final String CE_COMPATIBLE_RESPONSE = "ce_compatible_impl_method";
|
||||
private static final String CE_RESPONSE = "ce_impl_method";
|
||||
|
||||
@BeforeEach
|
||||
void setUp() {
|
||||
Mockito.when(featureFlagService.check(eq(FeatureFlagEnum.TENANT_TEST_FEATURE)))
|
||||
.thenReturn(Mono.just(false));
|
||||
|
||||
CachedFeatures cachedFeatures = new CachedFeatures();
|
||||
cachedFeatures.setFeatures(Map.of(FeatureFlagEnum.TENANT_TEST_FEATURE.name(), Boolean.FALSE));
|
||||
Mockito.when(featureFlagService.getCachedTenantFeatureFlags()).thenReturn(cachedFeatures);
|
||||
}
|
||||
|
||||
@Test
|
||||
void testEEOnlyMethod() {
|
||||
Mono<String> resultMono = testComponent.eeOnlyMethod();
|
||||
|
|
|
|||
|
|
@ -1,62 +0,0 @@
|
|||
package com.appsmith.server.featureflags.strategies;
|
||||
|
||||
import com.appsmith.server.domains.User;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.ff4j.core.FlippingExecutionContext;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.extension.ExtendWith;
|
||||
import org.mockito.Mockito;
|
||||
import org.springframework.boot.test.context.SpringBootTest;
|
||||
import org.springframework.security.test.context.support.WithUserDetails;
|
||||
import org.springframework.test.context.junit.jupiter.SpringExtension;
|
||||
|
||||
import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
@ExtendWith(SpringExtension.class)
|
||||
@SpringBootTest
|
||||
@Slf4j
|
||||
public class EmailBasedRolloutStrategyTest {
|
||||
|
||||
EmailBasedRolloutStrategy strategy = new EmailBasedRolloutStrategy();
|
||||
|
||||
@BeforeEach
|
||||
@WithUserDetails(value = "api_user")
|
||||
public void setup() {
|
||||
Map<String, String> initParams = new HashMap<>();
|
||||
initParams.put("emailDomains", "example.com,another-example.com");
|
||||
strategy.init("test-feature", initParams);
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithUserDetails(value = "api_user")
|
||||
public void testValidFeatureCheck() {
|
||||
FlippingExecutionContext executionContext = Mockito.mock(FlippingExecutionContext.class);
|
||||
|
||||
User user = new User();
|
||||
user.setEmail("test@EXAMPLE.com");
|
||||
Mockito.when(executionContext.getValue(Mockito.anyString(), Mockito.anyBoolean()))
|
||||
.thenReturn(user);
|
||||
|
||||
boolean evaluate = strategy.evaluate("test-feature", null, executionContext);
|
||||
assertTrue(evaluate);
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithUserDetails(value = "api_user")
|
||||
public void testInvalidFeatureCheck() {
|
||||
FlippingExecutionContext executionContext = Mockito.mock(FlippingExecutionContext.class);
|
||||
|
||||
User user = new User();
|
||||
user.setEmail("test@random.com");
|
||||
Mockito.when(executionContext.getValue(Mockito.anyString(), Mockito.anyBoolean()))
|
||||
.thenReturn(user);
|
||||
|
||||
boolean evaluate = strategy.evaluate("test-feature", null, executionContext);
|
||||
assertFalse(evaluate);
|
||||
}
|
||||
}
|
||||
|
|
@ -15,6 +15,11 @@ import reactor.core.publisher.Mono;
|
|||
|
||||
import java.time.Instant;
|
||||
import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
|
||||
import static com.appsmith.server.featureflags.FeatureFlagEnum.TENANT_TEST_FEATURE;
|
||||
import static com.appsmith.server.featureflags.FeatureFlagEnum.TEST_FEATURE_1;
|
||||
import static com.appsmith.server.featureflags.FeatureFlagEnum.TEST_FEATURE_2;
|
||||
|
||||
@Profile("test")
|
||||
@Primary
|
||||
|
|
@ -26,7 +31,10 @@ public class MockCacheableFeatureFlagHelper implements CacheableFeatureFlagHelpe
|
|||
public Mono<CachedFlags> fetchUserCachedFlags(String userIdentifier, User user) {
|
||||
CachedFlags cachedFlags = new CachedFlags();
|
||||
cachedFlags.setRefreshedAt(Instant.now());
|
||||
cachedFlags.setFlags(new HashMap<>());
|
||||
Map<String, Boolean> flags = new HashMap<>();
|
||||
flags.put(TEST_FEATURE_1.name(), true);
|
||||
flags.put(TEST_FEATURE_2.name(), false);
|
||||
cachedFlags.setFlags(flags);
|
||||
return Mono.just(cachedFlags);
|
||||
}
|
||||
|
||||
|
|
@ -68,7 +76,9 @@ public class MockCacheableFeatureFlagHelper implements CacheableFeatureFlagHelpe
|
|||
@Override
|
||||
public Mono<FeaturesResponseDTO> getRemoteFeaturesForTenant(FeaturesRequestDTO featuresRequestDTO) {
|
||||
FeaturesResponseDTO responseDTO = new FeaturesResponseDTO();
|
||||
responseDTO.setFeatures(new HashMap<>());
|
||||
Map<String, Boolean> features = new HashMap<>();
|
||||
features.put(TENANT_TEST_FEATURE.name(), true);
|
||||
responseDTO.setFeatures(features);
|
||||
return Mono.just(responseDTO);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,8 +11,6 @@ import com.appsmith.server.services.CacheableFeatureFlagHelper;
|
|||
import com.appsmith.server.services.FeatureFlagService;
|
||||
import com.appsmith.server.services.TenantService;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.ff4j.FF4j;
|
||||
import org.ff4j.conf.XmlParser;
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.Assertions;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
|
|
@ -21,10 +19,8 @@ import org.junit.jupiter.api.extension.ExtendWith;
|
|||
import org.mockito.Mockito;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.boot.test.context.SpringBootTest;
|
||||
import org.springframework.boot.test.context.TestConfiguration;
|
||||
import org.springframework.boot.test.mock.mockito.MockBean;
|
||||
import org.springframework.boot.test.mock.mockito.SpyBean;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.data.redis.core.ReactiveRedisTemplate;
|
||||
import org.springframework.security.test.context.support.WithUserDetails;
|
||||
import org.springframework.test.annotation.DirtiesContext;
|
||||
|
|
@ -45,7 +41,6 @@ import static com.appsmith.server.featureflags.FeatureFlagEnum.TENANT_TEST_FEATU
|
|||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertNotNull;
|
||||
import static org.junit.jupiter.api.Assertions.assertNull;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.ArgumentMatchers.anyString;
|
||||
|
|
@ -93,30 +88,7 @@ public class FeatureFlagServiceCETest {
|
|||
@WithUserDetails(value = "api_user")
|
||||
public void testNullFeatureCheck() {
|
||||
StepVerifier.create(featureFlagService.check(null))
|
||||
.assertNext(result -> {
|
||||
assertFalse(result);
|
||||
})
|
||||
.verifyComplete();
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithUserDetails(value = "api_user")
|
||||
public void testFeatureCheckForPonderationStrategy() {
|
||||
Math.random();
|
||||
StepVerifier.create(featureFlagService.check(FeatureFlagEnum.TEST_FEATURE_2))
|
||||
.assertNext(result -> {
|
||||
assertTrue(result);
|
||||
})
|
||||
.verifyComplete();
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithUserDetails(value = "api_user")
|
||||
public void testFeatureCheckForAppsmithUserStrategy() {
|
||||
StepVerifier.create(featureFlagService.check(FeatureFlagEnum.TEST_FEATURE_1))
|
||||
.assertNext(result -> {
|
||||
assertFalse(result);
|
||||
})
|
||||
.assertNext(Assertions::assertFalse)
|
||||
.verifyComplete();
|
||||
}
|
||||
|
||||
|
|
@ -131,17 +103,6 @@ public class FeatureFlagServiceCETest {
|
|||
.verifyComplete();
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithUserDetails(value = "api_user")
|
||||
public void testFeatureCheckForEmailStrategy() {
|
||||
StepVerifier.create(featureFlagService.getAllFeatureFlagsForUser())
|
||||
.assertNext(result -> {
|
||||
assertNotNull(result);
|
||||
assertTrue(result.containsKey(FeatureFlagEnum.TEST_FEATURE_3.toString()));
|
||||
})
|
||||
.verifyComplete();
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithUserDetails(value = "api_user")
|
||||
public void testGetFeaturesForUser_overrideWithTenantFeature() {
|
||||
|
|
@ -150,12 +111,12 @@ public class FeatureFlagServiceCETest {
|
|||
StepVerifier.create(featureFlagService.getAllFeatureFlagsForUser())
|
||||
.assertNext(result -> {
|
||||
assertNotNull(result);
|
||||
assertNull(result.get(TENANT_TEST_FEATURE.toString()));
|
||||
assertTrue(result.get(TENANT_TEST_FEATURE.toString()));
|
||||
})
|
||||
.verifyComplete();
|
||||
|
||||
Map<String, Boolean> tenantFeatures = new HashMap<>();
|
||||
tenantFeatures.put(TENANT_TEST_FEATURE.toString(), true);
|
||||
tenantFeatures.put(TENANT_TEST_FEATURE.toString(), false);
|
||||
FeaturesResponseDTO responseDTO = new FeaturesResponseDTO();
|
||||
responseDTO.setFeatures(tenantFeatures);
|
||||
doReturn(Mono.just(responseDTO)).when(cacheableFeatureFlagHelper).getRemoteFeaturesForTenant(any());
|
||||
|
|
@ -163,7 +124,7 @@ public class FeatureFlagServiceCETest {
|
|||
StepVerifier.create(featureFlagService.getAllFeatureFlagsForUser())
|
||||
.assertNext(result -> {
|
||||
assertNotNull(result);
|
||||
assertTrue(result.get(TENANT_TEST_FEATURE.toString()));
|
||||
assertFalse(result.get(TENANT_TEST_FEATURE.toString()));
|
||||
})
|
||||
.verifyComplete();
|
||||
}
|
||||
|
|
@ -175,9 +136,7 @@ public class FeatureFlagServiceCETest {
|
|||
Mono<CachedFlags> cachedFlagsMono = cacheableFeatureFlagHelper.fetchUserCachedFlags(userIdentifier, dummyUser);
|
||||
Mono<Boolean> hasKeyMono = reactiveRedisTemplate.hasKey("featureFlag:" + userIdentifier);
|
||||
StepVerifier.create(cachedFlagsMono.then(hasKeyMono))
|
||||
.assertNext(isKeyPresent -> {
|
||||
assertTrue(isKeyPresent);
|
||||
})
|
||||
.assertNext(Assertions::assertTrue)
|
||||
.verifyComplete();
|
||||
}
|
||||
|
||||
|
|
@ -187,9 +146,7 @@ public class FeatureFlagServiceCETest {
|
|||
Mono<Void> evictCache = cacheableFeatureFlagHelper.evictUserCachedFlags(userIdentifier);
|
||||
Mono<Boolean> hasKeyMono = reactiveRedisTemplate.hasKey("featureFlag:" + userIdentifier);
|
||||
StepVerifier.create(evictCache.then(hasKeyMono))
|
||||
.assertNext(isKeyPresent -> {
|
||||
assertFalse(isKeyPresent);
|
||||
})
|
||||
.assertNext(Assertions::assertFalse)
|
||||
.verifyComplete();
|
||||
}
|
||||
|
||||
|
|
@ -324,34 +281,23 @@ public class FeatureFlagServiceCETest {
|
|||
|
||||
// Assert that the cached feature flags are empty before the remote fetch
|
||||
CachedFeatures cachedFeaturesBeforeRemoteCall = featureFlagService.getCachedTenantFeatureFlags();
|
||||
assertTrue(cachedFeaturesBeforeRemoteCall.getFeatures().isEmpty());
|
||||
assertThat(cachedFeaturesBeforeRemoteCall.getFeatures().size()).isEqualTo(1);
|
||||
assertTrue(cachedFeaturesBeforeRemoteCall.getFeatures().get(TENANT_TEST_FEATURE.name()));
|
||||
|
||||
Map<String, Boolean> tenantFeatures = new HashMap<>();
|
||||
tenantFeatures.put(TENANT_TEST_FEATURE.name(), true);
|
||||
tenantFeatures.put(TENANT_TEST_FEATURE.name(), false);
|
||||
FeaturesResponseDTO responseDTO = new FeaturesResponseDTO();
|
||||
responseDTO.setFeatures(tenantFeatures);
|
||||
doReturn(Mono.just(responseDTO)).when(cacheableFeatureFlagHelper).getRemoteFeaturesForTenant(any());
|
||||
StepVerifier.create(featureFlagService.getTenantFeatures())
|
||||
.assertNext(result -> {
|
||||
assertNotNull(result);
|
||||
assertTrue(result.get(TENANT_TEST_FEATURE.name()));
|
||||
assertFalse(result.get(TENANT_TEST_FEATURE.name()));
|
||||
|
||||
// Check if the cached feature flags are updated after the remote fetch
|
||||
CachedFeatures cachedFeaturesAfterRemoteCall = featureFlagService.getCachedTenantFeatureFlags();
|
||||
assertTrue(cachedFeaturesAfterRemoteCall.getFeatures().get(TENANT_TEST_FEATURE.name()));
|
||||
assertFalse(cachedFeaturesAfterRemoteCall.getFeatures().get(TENANT_TEST_FEATURE.name()));
|
||||
})
|
||||
.verifyComplete();
|
||||
}
|
||||
|
||||
@TestConfiguration
|
||||
static class TestFeatureFlagConfig {
|
||||
|
||||
@Bean
|
||||
FF4j ff4j() {
|
||||
FF4j ff4j = new FF4j(new XmlParser(), "features/init-flags-test.xml")
|
||||
.audit(true)
|
||||
.autoCreate(false);
|
||||
return ff4j;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,24 +0,0 @@
|
|||
<?xml version="1.0" encoding="UTF-8" ?>
|
||||
<ff4j>
|
||||
<autocreate>false</autocreate>
|
||||
<audit>false</audit>
|
||||
<features>
|
||||
<feature uid="TEST_FEATURE_1" enable="true"
|
||||
description="The test feature should only be visible to Appsmith users">
|
||||
<flipstrategy class="com.appsmith.server.featureflags.strategies.AppsmithUserStrategy">
|
||||
<param name="requiredKey" value="requiredValue"/>
|
||||
</flipstrategy>
|
||||
</feature>
|
||||
<feature uid="TEST_FEATURE_2" enable="true" description="Enable this feature based on weight. It's randomized.">
|
||||
<flipstrategy class="org.ff4j.strategy.PonderationStrategy">
|
||||
<param name="weight" value="1"/>
|
||||
</flipstrategy>
|
||||
</feature>
|
||||
<feature uid="TEST_FEATURE_3" enable="true"
|
||||
description="The test feature should only be visible to certain users">
|
||||
<flipstrategy class="com.appsmith.server.featureflags.strategies.EmailBasedRolloutStrategy">
|
||||
<param name="emailDomains" value="test@example.com"/>
|
||||
</flipstrategy>
|
||||
</feature>
|
||||
</features>
|
||||
</ff4j>
|
||||
Loading…
Reference in New Issue
Block a user