Protei/PromucFlow_constructor
2
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

refactor: Remove strong dependency from feature flag enum to (de)serialise organization object (#40374)

Browse Source
This commit is contained in:
Abhijeet 2025-04-28 14:44:43 +05:30 committed by GitHub
parent 6e715881a8
commit c14a896a7a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 61 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/OrganizationConfigurationCE.java
View File

@ -1,6 +1,5 @@
package com.appsmith.server.domains.ce;
import com.appsmith.external.enums.FeatureFlagEnum;
import com.appsmith.server.constants.FeatureMigrationType;
import com.appsmith.server.constants.LicensePlan;
import com.appsmith.server.constants.MigrationStatus;
@ -58,7 +57,7 @@ public class OrganizationConfigurationCE implements Serializable {
// the feature flags. This can happen for 2 reasons:
// 1. The license plan changes
// 2. Because of grandfathering via cron where organization level feature flags are fetched
Map<FeatureFlagEnum, FeatureMigrationType> featuresWithPendingMigration;
Map<String, FeatureMigrationType> featuresWithPendingMigration;
Boolean isStrongPasswordPolicyEnabled;

5
app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/FeatureFlagMigrationHelperCE.java
View File

@ -9,10 +9,9 @@ import java.util.Map;
public interface FeatureFlagMigrationHelperCE {
Mono<Map<FeatureFlagEnum, FeatureMigrationType>> getUpdatedFlagsWithPendingMigration(
Organization defaultOrganization);
Mono<Map<String, FeatureMigrationType>> getUpdatedFlagsWithPendingMigration(Organization defaultOrganization);
Mono<Map<FeatureFlagEnum, FeatureMigrationType>> getUpdatedFlagsWithPendingMigration(
Mono<Map<String, FeatureMigrationType>> getUpdatedFlagsWithPendingMigration(
Organization organization, boolean forceUpdate);
Mono<Boolean> checkAndExecuteMigrationsForFeatureFlag(Organization organization, FeatureFlagEnum featureFlagEnum);

51
app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/FeatureFlagMigrationHelperCEImpl.java
View File

@ -7,6 +7,7 @@ import com.appsmith.server.domains.OrganizationConfiguration;
import com.appsmith.server.featureflags.CachedFeatures;
import com.appsmith.server.helpers.CollectionUtils;
import com.appsmith.server.services.CacheableFeatureFlagHelper;
import com.appsmith.server.solutions.ce.ScheduledTaskCEImpl;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import reactor.core.publisher.Mono;
@ -36,7 +37,7 @@ public class FeatureFlagMigrationHelperCEImpl implements FeatureFlagMigrationHel
private static final long ORGANIZATION_FEATURES_CACHE_TIME_MIN = 115;
@Override
public Mono<Map<FeatureFlagEnum, FeatureMigrationType>> getUpdatedFlagsWithPendingMigration(
public Mono<Map<String, FeatureMigrationType>> getUpdatedFlagsWithPendingMigration(
Organization defaultOrganization) {
return getUpdatedFlagsWithPendingMigration(defaultOrganization, FALSE);
}
@ -49,7 +50,7 @@ public class FeatureFlagMigrationHelperCEImpl implements FeatureFlagMigrationHel
* @return Map of feature flags with pending migrations
*/
@Override
public Mono<Map<FeatureFlagEnum, FeatureMigrationType>> getUpdatedFlagsWithPendingMigration(
public Mono<Map<String, FeatureMigrationType>> getUpdatedFlagsWithPendingMigration(
Organization organization, boolean forceUpdate) {
/*
@ -127,50 +128,43 @@ public class FeatureFlagMigrationHelperCEImpl implements FeatureFlagMigrationHel
* @param existingCachedFlags Flags which are already stored in cache
* @return updated organization with the required flags with pending migrations
*/
private Map<FeatureFlagEnum, FeatureMigrationType> getUpdatedFlagsWithPendingMigration(
private Map<String, FeatureMigrationType> getUpdatedFlagsWithPendingMigration(
Organization organization, CachedFeatures latestFlags, CachedFeatures existingCachedFlags) {
// 1. Check if there are any diffs for the feature flags
// 2. Update the flags for pending migration within provided organization object
Map<FeatureFlagEnum, FeatureMigrationType> featureDiffsWithMigrationType = new HashMap<>();
Map<String, FeatureMigrationType> featureDiffsWithMigrationType = new HashMap<>();
Map<String, Boolean> existingFeatureMap = existingCachedFlags.getFeatures();
latestFlags.getFeatures().forEach((key, value) -> {
if (value != null && !value.equals(existingFeatureMap.get(key))) {
try {
featureDiffsWithMigrationType.put(
FeatureFlagEnum.valueOf(key), Boolean.TRUE.equals(value) ? ENABLE : DISABLE);
featureDiffsWithMigrationType.put(key, Boolean.TRUE.equals(value) ? ENABLE : DISABLE);
} catch (Exception e) {
// Ignore IllegalArgumentException as all the feature flags are not added on
// server side
if (!(e instanceof IllegalArgumentException)) {
log.error("Error while parsing the feature flag {} with value {}", key, value, e);
}
log.error("Error while processing the feature flag {} with value {}", key, value, e);
}
}
});
return getUpdatedFlagsWithPendingMigration(featureDiffsWithMigrationType, organization);
}
private Map<FeatureFlagEnum, FeatureMigrationType> getUpdatedFlagsWithPendingMigration(
Map<FeatureFlagEnum, FeatureMigrationType> latestFeatureDiffsWithMigrationType,
Organization dbOrganization) {
private Map<String, FeatureMigrationType> getUpdatedFlagsWithPendingMigration(
Map<String, FeatureMigrationType> latestFeatureDiffsWithMigrationType, Organization dbOrganization) {
Map<FeatureFlagEnum, FeatureMigrationType> featuresWithPendingMigrationDB =
Map<String, FeatureMigrationType> featuresWithPendingMigrationDB =
dbOrganization.getOrganizationConfiguration().getFeaturesWithPendingMigration() == null
? new HashMap<>()
: dbOrganization.getOrganizationConfiguration().getFeaturesWithPendingMigration();
Map<FeatureFlagEnum, FeatureMigrationType> updatedFlagsForMigrations =
new HashMap<>(featuresWithPendingMigrationDB);
Map<String, FeatureMigrationType> updatedFlagsForMigrations = new HashMap<>(featuresWithPendingMigrationDB);
// We should expect the following state after the latest run:
// featuresWithPendingMigrationDB => {feature1 : enable, feature2 : disable}
// latestFeatureDiffsWithMigrationType => {feature1 : enable, feature2 : enable, feature3 : disable}
// updatedFlagsForMigrations => {feature1 : enable, feature3 : disable}
List<FeatureFlagEnum> featureFlagsToBeRemoved = new ArrayList<>();
updatedFlagsForMigrations.forEach((featureFlagEnum, featureMigrationType) -> {
if (latestFeatureDiffsWithMigrationType.containsKey(featureFlagEnum)
&& !featureMigrationType.equals(latestFeatureDiffsWithMigrationType.get(featureFlagEnum))) {
List<String> featureFlagsToBeRemoved = new ArrayList<>();
updatedFlagsForMigrations.forEach((featureFlag, featureMigrationType) -> {
if (latestFeatureDiffsWithMigrationType.containsKey(featureFlag)
&& !featureMigrationType.equals(latestFeatureDiffsWithMigrationType.get(featureFlag))) {
/*
Scenario when the migrations will be blocked on user input and may end up in a case where we just have
to remove the entry as migration is no longer needed:
@ -182,7 +176,7 @@ public class FeatureFlagMigrationHelperCEImpl implements FeatureFlagMigrationHel
Step 4: User adds the valid key or renews the subscription again which results in enabling the
feature and ends up in nullifying the effect for step 2
*/
featureFlagsToBeRemoved.add(featureFlagEnum);
featureFlagsToBeRemoved.add(featureFlag);
}
});
@ -217,16 +211,16 @@ public class FeatureFlagMigrationHelperCEImpl implements FeatureFlagMigrationHel
return Mono.just(TRUE);
}
Map<FeatureFlagEnum, FeatureMigrationType> featuresWithPendingMigration =
Map<String, FeatureMigrationType> featuresWithPendingMigration =
organizationConfiguration.getFeaturesWithPendingMigration();
if (CollectionUtils.isNullOrEmpty(featuresWithPendingMigration)
|| !featuresWithPendingMigration.containsKey(featureFlagEnum)) {
|| !featuresWithPendingMigration.containsKey(featureFlagEnum.name())) {
return Mono.just(TRUE);
}
log.debug(
"Running the migration for flag {} with migration type {}",
featureFlagEnum.name(),
featuresWithPendingMigration.get(featureFlagEnum));
featuresWithPendingMigration.get(featureFlagEnum.name()));
return this.executeMigrationsBasedOnFeatureFlag(organization, featureFlagEnum);
});
}
@ -238,7 +232,7 @@ public class FeatureFlagMigrationHelperCEImpl implements FeatureFlagMigrationHel
* @return Boolean indicating if the migrations is required or not
*/
private Mono<Boolean> isMigrationRequired(Organization organization, FeatureFlagEnum featureFlagEnum) {
Map<FeatureFlagEnum, FeatureMigrationType> featureMigrationTypeMap =
Map<String, FeatureMigrationType> featureMigrationTypeMap =
organization.getOrganizationConfiguration().getFeaturesWithPendingMigration();
if (CollectionUtils.isNullOrEmpty(featureMigrationTypeMap)) {
return Mono.just(FALSE);
@ -250,10 +244,10 @@ public class FeatureFlagMigrationHelperCEImpl implements FeatureFlagMigrationHel
if (featureFlags.containsKey(featureFlagEnum.name())) {
return (TRUE.equals(featureFlags.get(featureFlagEnum.name()))
&& FeatureMigrationType.ENABLE.equals(
featureMigrationTypeMap.get(featureFlagEnum)))
featureMigrationTypeMap.get(featureFlagEnum.name())))
|| (FALSE.equals(featureFlags.get(featureFlagEnum.name()))
&& FeatureMigrationType.DISABLE.equals(
featureMigrationTypeMap.get(featureFlagEnum)));
featureMigrationTypeMap.get(featureFlagEnum.name())));
}
return FALSE;
});
@ -268,6 +262,7 @@ public class FeatureFlagMigrationHelperCEImpl implements FeatureFlagMigrationHel
@Override
public Mono<Boolean> executeMigrationsBasedOnFeatureFlag(
Organization organization, FeatureFlagEnum featureFlagEnum) {
// Placeholder implementation for extending classes
return Mono.just(TRUE);
}
}

21
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCEImpl.java
View File

@ -290,16 +290,27 @@ public class OrganizationServiceCEImpl extends BaseService<OrganizationRepositor
if (!isMigrationRequired(organization)) {
return Mono.just(organization);
}
Map<FeatureFlagEnum, FeatureMigrationType> featureMigrationTypeMap =
Map<String, FeatureMigrationType> featureMigrationTypeMap =
organization.getOrganizationConfiguration().getFeaturesWithPendingMigration();
FeatureFlagEnum featureFlagEnum =
final String featureFlagKey =
featureMigrationTypeMap.keySet().stream().findFirst().orElse(null);
FeatureFlagEnum featureFlagEnum = null;
if (featureFlagKey != null) {
try {
featureFlagEnum = FeatureFlagEnum.valueOf(featureFlagKey);
} catch (IllegalArgumentException e) {
log.warn("Unknown feature flag: {}", featureFlagKey);
}
}
final FeatureFlagEnum finalFeatureFlagEnum = featureFlagEnum;
return featureFlagMigrationHelper
.checkAndExecuteMigrationsForFeatureFlag(organization, featureFlagEnum)
.checkAndExecuteMigrationsForFeatureFlag(organization, finalFeatureFlagEnum)
.flatMap(isSuccessful -> {
if (TRUE.equals(isSuccessful)) {
featureMigrationTypeMap.remove(featureFlagEnum);
featureMigrationTypeMap.remove(featureFlagKey);
if (CollectionUtils.isNullOrEmpty(featureMigrationTypeMap)) {
organization.getOrganizationConfiguration().setMigrationStatus(MigrationStatus.COMPLETED);
} else {
@ -312,7 +323,7 @@ public class OrganizationServiceCEImpl extends BaseService<OrganizationRepositor
.flatMap(this::checkAndExecuteMigrationsForOrganizationFeatureFlags);
}
return Mono.error(
new AppsmithException(AppsmithError.FeatureFlagMigrationFailure, featureFlagEnum, ""));
new AppsmithException(AppsmithError.FeatureFlagMigrationFailure, finalFeatureFlagEnum, ""));
});
}

19
app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/FeatureFlagMigrationHelperTest.java
View File

@ -1,6 +1,5 @@
package com.appsmith.server.helpers;
import com.appsmith.external.enums.FeatureFlagEnum;
import com.appsmith.server.constants.FeatureMigrationType;
import com.appsmith.server.domains.Organization;
import com.appsmith.server.domains.OrganizationConfiguration;
@ -65,14 +64,14 @@ class FeatureFlagMigrationHelperTest {
Mockito.when(cacheableFeatureFlagHelper.evictCachedOrganizationFeatures(any()))
.thenReturn(Mono.empty());
Mono<Map<FeatureFlagEnum, FeatureMigrationType>> getUpdatedFlagsWithPendingMigration =
Mono<Map<String, FeatureMigrationType>> getUpdatedFlagsWithPendingMigration =
featureFlagMigrationHelper.getUpdatedFlagsWithPendingMigration(defaultOrganization);
StepVerifier.create(getUpdatedFlagsWithPendingMigration)
.assertNext(featureFlagEnumFeatureMigrationTypeMap -> {
assertThat(featureFlagEnumFeatureMigrationTypeMap).isNotEmpty();
assertThat(featureFlagEnumFeatureMigrationTypeMap).hasSize(1);
assertThat(featureFlagEnumFeatureMigrationTypeMap.get(ORGANIZATION_TEST_FEATURE))
assertThat(featureFlagEnumFeatureMigrationTypeMap.get(ORGANIZATION_TEST_FEATURE.name()))
.isEqualTo(DISABLE);
})
.verifyComplete();
@ -103,14 +102,14 @@ class FeatureFlagMigrationHelperTest {
Mockito.when(cacheableFeatureFlagHelper.evictCachedOrganizationFeatures(any()))
.thenReturn(Mono.empty());
Mono<Map<FeatureFlagEnum, FeatureMigrationType>> getUpdatedFlagsWithPendingMigration =
Mono<Map<String, FeatureMigrationType>> getUpdatedFlagsWithPendingMigration =
featureFlagMigrationHelper.getUpdatedFlagsWithPendingMigration(defaultOrganization);
StepVerifier.create(getUpdatedFlagsWithPendingMigration)
.assertNext(featureFlagEnumFeatureMigrationTypeMap -> {
assertThat(featureFlagEnumFeatureMigrationTypeMap).isNotEmpty();
assertThat(featureFlagEnumFeatureMigrationTypeMap).hasSize(1);
assertThat(featureFlagEnumFeatureMigrationTypeMap.get(ORGANIZATION_TEST_FEATURE))
assertThat(featureFlagEnumFeatureMigrationTypeMap.get(ORGANIZATION_TEST_FEATURE.name()))
.isEqualTo(ENABLE);
})
.verifyComplete();
@ -134,7 +133,7 @@ class FeatureFlagMigrationHelperTest {
Mockito.when(cacheableFeatureFlagHelper.evictCachedOrganizationFeatures(any()))
.thenReturn(Mono.empty());
Mono<Map<FeatureFlagEnum, FeatureMigrationType>> getUpdatedFlagsWithPendingMigration =
Mono<Map<String, FeatureMigrationType>> getUpdatedFlagsWithPendingMigration =
featureFlagMigrationHelper.getUpdatedFlagsWithPendingMigration(defaultOrganization);
StepVerifier.create(getUpdatedFlagsWithPendingMigration)
@ -172,7 +171,7 @@ class FeatureFlagMigrationHelperTest {
Mockito.when(cacheableFeatureFlagHelper.evictCachedOrganizationFeatures(any()))
.thenReturn(Mono.empty());
Mono<Map<FeatureFlagEnum, FeatureMigrationType>> getUpdatedFlagsWithPendingMigration =
Mono<Map<String, FeatureMigrationType>> getUpdatedFlagsWithPendingMigration =
featureFlagMigrationHelper.getUpdatedFlagsWithPendingMigration(defaultOrganization);
StepVerifier.create(getUpdatedFlagsWithPendingMigration)
@ -198,7 +197,7 @@ class FeatureFlagMigrationHelperTest {
void checkAndExecuteMigrationsForFeatureFlag_validFeatureFlag_success() {
Organization defaultOrganization = new Organization();
OrganizationConfiguration organizationConfiguration = new OrganizationConfiguration();
organizationConfiguration.setFeaturesWithPendingMigration(Map.of(ORGANIZATION_TEST_FEATURE, ENABLE));
organizationConfiguration.setFeaturesWithPendingMigration(Map.of(ORGANIZATION_TEST_FEATURE.name(), ENABLE));
organizationConfiguration.setMigrationStatus(PENDING);
defaultOrganization.setOrganizationConfiguration(organizationConfiguration);
@ -231,7 +230,7 @@ class FeatureFlagMigrationHelperTest {
Organization defaultOrganization = new Organization();
defaultOrganization.setId(UUID.randomUUID().toString());
OrganizationConfiguration organizationConfiguration = new OrganizationConfiguration();
organizationConfiguration.setFeaturesWithPendingMigration(Map.of(ORGANIZATION_TEST_FEATURE, DISABLE));
organizationConfiguration.setFeaturesWithPendingMigration(Map.of(ORGANIZATION_TEST_FEATURE.name(), DISABLE));
defaultOrganization.setOrganizationConfiguration(organizationConfiguration);
// Mock CS calls to fetch the feature flags to have the feature flag in pending migration list with ENABLE
@ -256,7 +255,7 @@ class FeatureFlagMigrationHelperTest {
Mockito.when(cacheableFeatureFlagHelper.evictCachedOrganizationFeatures(any()))
.thenReturn(Mono.empty());
Mono<Map<FeatureFlagEnum, FeatureMigrationType>> getUpdatedFlagsWithPendingMigration =
Mono<Map<String, FeatureMigrationType>> getUpdatedFlagsWithPendingMigration =
featureFlagMigrationHelper.getUpdatedFlagsWithPendingMigration(defaultOrganization);
StepVerifier.create(getUpdatedFlagsWithPendingMigration)

8
app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/FeatureFlagServiceCETest.java
View File

@ -226,7 +226,7 @@ public class FeatureFlagServiceCETest {
getAllRemoteFeaturesForOrganizationAndUpdateFeatureFlagsWithPendingMigrations_disableMigration_statesUpdate() {
Mockito.when(featureFlagMigrationHelper.getUpdatedFlagsWithPendingMigration(any()))
.thenReturn(Mono.just(Map.of(ORGANIZATION_TEST_FEATURE, DISABLE)));
.thenReturn(Mono.just(Map.of(ORGANIZATION_TEST_FEATURE.name(), DISABLE)));
organizationService
.retrieveAll()
@ -237,7 +237,7 @@ public class FeatureFlagServiceCETest {
StepVerifier.create(organizationService.getCurrentUserOrganization())
.assertNext(organization -> {
assertThat(organization.getOrganizationConfiguration().getFeaturesWithPendingMigration())
.isEqualTo(Map.of(ORGANIZATION_TEST_FEATURE, DISABLE));
.isEqualTo(Map.of(ORGANIZATION_TEST_FEATURE.name(), DISABLE));
assertThat(organization.getOrganizationConfiguration().getMigrationStatus())
.isEqualTo(PENDING);
})
@ -249,7 +249,7 @@ public class FeatureFlagServiceCETest {
getAllRemoteFeaturesForOrganizationAndUpdateFeatureFlagsWithPendingMigrations_enableMigration_statesUpdate() {
Mockito.when(featureFlagMigrationHelper.getUpdatedFlagsWithPendingMigration(any()))
.thenReturn(Mono.just(Map.of(ORGANIZATION_TEST_FEATURE, ENABLE)));
.thenReturn(Mono.just(Map.of(ORGANIZATION_TEST_FEATURE.name(), ENABLE)));
organizationService
.retrieveAll()
@ -260,7 +260,7 @@ public class FeatureFlagServiceCETest {
StepVerifier.create(organizationService.getCurrentUserOrganization())
.assertNext(organization -> {
assertThat(organization.getOrganizationConfiguration().getFeaturesWithPendingMigration())
.isEqualTo(Map.of(ORGANIZATION_TEST_FEATURE, ENABLE));
.isEqualTo(Map.of(ORGANIZATION_TEST_FEATURE.name(), ENABLE));
assertThat(organization.getOrganizationConfiguration().getMigrationStatus())
.isEqualTo(PENDING);
})

13
app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/OrganizationServiceCETest.java
View File

@ -1,7 +1,6 @@
package com.appsmith.server.services.ce;
import com.appsmith.caching.components.CacheManager;
import com.appsmith.external.enums.FeatureFlagEnum;
import com.appsmith.server.constants.FeatureMigrationType;
import com.appsmith.server.constants.LicensePlan;
import com.appsmith.server.domains.Organization;
@ -339,10 +338,10 @@ class OrganizationServiceCETest {
organization.setId(UUID.randomUUID().toString());
organization.setSlug(UUID.randomUUID().toString());
OrganizationConfiguration config = new OrganizationConfiguration();
Map<FeatureFlagEnum, FeatureMigrationType> featureMigrationTypeMap = new HashMap<>();
Map<String, FeatureMigrationType> featureMigrationTypeMap = new HashMap<>();
config.setFeaturesWithPendingMigration(featureMigrationTypeMap);
featureMigrationTypeMap.put(ORGANIZATION_TEST_FEATURE, FeatureMigrationType.ENABLE);
featureMigrationTypeMap.put(TEST_FEATURE_2, FeatureMigrationType.DISABLE);
featureMigrationTypeMap.put(ORGANIZATION_TEST_FEATURE.name(), FeatureMigrationType.ENABLE);
featureMigrationTypeMap.put(TEST_FEATURE_2.name(), FeatureMigrationType.DISABLE);
organization.setOrganizationConfiguration(config);
final Mono<Organization> resultMono =
organizationService.checkAndExecuteMigrationsForOrganizationFeatureFlags(organization);
@ -368,10 +367,10 @@ class OrganizationServiceCETest {
organization.setId(UUID.randomUUID().toString());
organization.setSlug(UUID.randomUUID().toString());
OrganizationConfiguration config = new OrganizationConfiguration();
Map<FeatureFlagEnum, FeatureMigrationType> featureMigrationTypeMap = new HashMap<>();
Map<String, FeatureMigrationType> featureMigrationTypeMap = new HashMap<>();
config.setFeaturesWithPendingMigration(featureMigrationTypeMap);
featureMigrationTypeMap.put(ORGANIZATION_TEST_FEATURE, FeatureMigrationType.DISABLE);
featureMigrationTypeMap.put(TEST_FEATURE_2, FeatureMigrationType.ENABLE);
featureMigrationTypeMap.put(ORGANIZATION_TEST_FEATURE.name(), FeatureMigrationType.DISABLE);
featureMigrationTypeMap.put(TEST_FEATURE_2.name(), FeatureMigrationType.ENABLE);
organization.setOrganizationConfiguration(config);
final Mono<Organization> resultMono =
organizationService.checkAndExecuteMigrationsForOrganizationFeatureFlags(organization);