diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/OrganizationConfigurationCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/OrganizationConfigurationCE.java index d6e025abec85..afd543933c09 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/OrganizationConfigurationCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/OrganizationConfigurationCE.java @@ -10,6 +10,7 @@ import lombok.Data; import lombok.experimental.FieldNameConstants; import org.apache.commons.lang3.ObjectUtils; +import org.springframework.data.annotation.Transient; import java.io.Serializable; import java.util.ArrayList; @@ -20,17 +21,20 @@ @FieldNameConstants public class OrganizationConfigurationCE implements Serializable { + @Transient @Deprecated(forRemoval = true, since = "v1.65") private String googleMapsKey; private Boolean isFormLoginEnabled; + @Transient @Deprecated(forRemoval = true, since = "v1.65") private String instanceName; protected License license; // organization admin can toggle this field to enable/disable email verification + @Transient @Deprecated(forRemoval = true, since = "v1.65") private Boolean emailVerificationEnabled; @@ -82,7 +86,7 @@ public void copyNonSensitiveValues(OrganizationConfiguration organizationConfigu ObjectUtils.defaultIfNull(organizationConfiguration.getIsFormLoginEnabled(), isFormLoginEnabled); instanceName = ObjectUtils.defaultIfNull(organizationConfiguration.getInstanceName(), instanceName); emailVerificationEnabled = ObjectUtils.defaultIfNull( - organizationConfiguration.isEmailVerificationEnabled(), emailVerificationEnabled); + organizationConfiguration.getEmailVerificationEnabled(), emailVerificationEnabled); featuresWithPendingMigration = organizationConfiguration.getFeaturesWithPendingMigration(); migrationStatus = organizationConfiguration.getMigrationStatus(); @@ -90,10 +94,6 @@ public void copyNonSensitiveValues(OrganizationConfiguration organizationConfigu isAtomicPushAllowed = organizationConfiguration.getIsAtomicPushAllowed(); } - public Boolean isEmailVerificationEnabled() { - return Boolean.TRUE.equals(this.emailVerificationEnabled); - } - public static class Fields { public Fields() { // Public constructor for Fields class diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/InstanceVariablesHelperCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/InstanceVariablesHelperCE.java index efd6d0ccd5d1..5ecd1bcf2281 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/InstanceVariablesHelperCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/InstanceVariablesHelperCE.java @@ -1,10 +1,15 @@ package com.appsmith.server.helpers.ce; import com.appsmith.server.constants.Appsmith; +import com.appsmith.server.domains.OrganizationConfiguration; import com.appsmith.server.services.ConfigService; import lombok.RequiredArgsConstructor; +import org.springframework.util.StringUtils; import reactor.core.publisher.Mono; +import java.util.HashMap; +import java.util.Map; + /** * Helper class for accessing instance variables from the instance config */ @@ -47,4 +52,47 @@ public Mono getGoogleMapsKey() { return value != null ? value.toString() : ""; }); } + + public OrganizationConfiguration populateOrgConfigWithInstanceVariables( + Map instanceVariables, OrganizationConfiguration organizationConfiguration) { + Object value = instanceVariables.getOrDefault("instanceName", Appsmith.DEFAULT_INSTANCE_NAME); + organizationConfiguration.setInstanceName(value != null ? value.toString() : Appsmith.DEFAULT_INSTANCE_NAME); + + value = instanceVariables.getOrDefault("emailVerificationEnabled", false); + if (value instanceof Boolean) { + organizationConfiguration.setEmailVerificationEnabled((Boolean) value); + } else { + organizationConfiguration.setEmailVerificationEnabled(Boolean.FALSE); + } + + value = instanceVariables.getOrDefault("googleMapsKey", ""); + organizationConfiguration.setGoogleMapsKey(value != null ? value.toString() : ""); + + return organizationConfiguration; + } + + // TODO @CloudBilling: Temporary method to update instance variables via organization configuration. This method + // will be removed once the instance variables will be removed from organization configuration + public Mono updateInstanceVariables(OrganizationConfiguration orgConfig) { + + Map updatedInstanceVariables = updateAllowedInstanceVariables(orgConfig); + return configService.getInstanceVariables().flatMap(instanceVariable -> { + instanceVariable.putAll(updatedInstanceVariables); + return configService.updateInstanceVariables(instanceVariable).thenReturn(orgConfig); + }); + } + + protected Map updateAllowedInstanceVariables(OrganizationConfiguration orgConfig) { + Map instanceVariables = new HashMap<>(); + if (StringUtils.hasLength(orgConfig.getInstanceName())) { + instanceVariables.put("instanceName", orgConfig.getInstanceName()); + } + if (orgConfig.getEmailVerificationEnabled() != null) { + instanceVariables.put("emailVerificationEnabled", orgConfig.getEmailVerificationEnabled()); + } + if (StringUtils.hasLength(orgConfig.getGoogleMapsKey())) { + instanceVariables.put("googleMapsKey", orgConfig.getGoogleMapsKey()); + } + return instanceVariables; + } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration069MigrateOrganizationConfigToInstanceConfig.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration069MigrateOrganizationConfigToInstanceConfig.java index 194a22a4903d..67237bb6d2cc 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration069MigrateOrganizationConfigToInstanceConfig.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/db/ce/Migration069MigrateOrganizationConfigToInstanceConfig.java @@ -70,7 +70,7 @@ public void migrateOrganizationConfigToInstanceConfig() { instanceVariables.put("instanceName", Appsmith.DEFAULT_INSTANCE_NAME); } - instanceVariables.put("emailVerificationEnabled", orgConfig.isEmailVerificationEnabled()); + instanceVariables.put("emailVerificationEnabled", Boolean.TRUE.equals(orgConfig.getEmailVerificationEnabled())); instanceVariables.put("googleMapsKey", orgConfig.getGoogleMapsKey()); // Add instanceVariables to config diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ConfigServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ConfigServiceCE.java index 0b7c98252923..1f4644a5b25c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ConfigServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ConfigServiceCE.java @@ -3,7 +3,6 @@ import com.appsmith.server.acl.AclPermission; import com.appsmith.server.domains.Config; import com.appsmith.server.domains.User; -import net.minidev.json.JSONObject; import reactor.core.publisher.Mono; import java.util.Map; @@ -37,5 +36,5 @@ public interface ConfigServiceCE { * @param instanceVariables JSONObject containing the instance variables to update * @return Updated Config object */ - Mono updateInstanceVariables(JSONObject instanceVariables); + Mono updateInstanceVariables(Map instanceVariables); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ConfigServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ConfigServiceCEImpl.java index 0f1b2a8776b8..fb544da3b901 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ConfigServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ConfigServiceCEImpl.java @@ -114,13 +114,13 @@ public Mono> getInstanceVariables() { } @Override - public Mono updateInstanceVariables(JSONObject instanceVariables) { - return getByName(FieldName.INSTANCE_CONFIG, AclPermission.MANAGE_INSTANCE_CONFIGURATION) - .flatMap(config -> { - JSONObject configObj = config.getConfig(); - configObj.put(INSTANCE_VARIABLES, instanceVariables); - config.setConfig(configObj); - return repository.save(config); - }); + public Mono updateInstanceVariables(Map instanceVariables) { + // TODO @CloudBilling add manage instance permission once the migration for variables is complete + return getByName(FieldName.INSTANCE_CONFIG).flatMap(config -> { + JSONObject configObj = config.getConfig(); + configObj.put(INSTANCE_VARIABLES, instanceVariables); + config.setConfig(configObj); + return repository.save(config); + }); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCEImpl.java index 32591aa61a0b..53ca8bf93420 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCEImpl.java @@ -103,7 +103,7 @@ public Mono updateOrganizationConfiguration( } Mono> envMono = Mono.empty(); // instance admin is setting the email verification to true but the SMTP settings are not configured - if (organizationConfiguration.isEmailVerificationEnabled() == Boolean.TRUE) { + if (Boolean.TRUE.equals(organizationConfiguration.getEmailVerificationEnabled())) { envMono = envManager.getAllNonEmpty().flatMap(properties -> { String mailHost = properties.get("APPSMITH_MAIL_HOST"); if (mailHost == null || mailHost == "") { @@ -128,10 +128,12 @@ public Mono updateOrganizationConfiguration( Mono updatedOrganizationMono = repository .updateById(organizationId, organization, MANAGE_ORGANIZATION) .cache(); + // Firstly updating the Organization object in the database and then evicting the cache. // returning the updatedOrganization, notice the updatedOrganizationMono is cached using .cache() // hence it will not be evaluated again return updatedOrganizationMono + .then(instanceVariablesHelper.updateInstanceVariables(organizationConfiguration)) .then(Mono.defer(() -> evictOrganizationCache)) .then(Mono.defer(() -> allSideEffectsMono)) .then(updatedOrganizationMono); @@ -154,12 +156,10 @@ public Mono findById(String organizationId, AclPermission permissi @Override public Mono getOrganizationConfiguration(Mono dbOrganizationMono) { String adminEmailDomainHash = commonConfig.getAdminEmailDomainHash(); - Mono clientOrganizationMono = Mono.zip( - configService.getInstanceId(), instanceVariablesHelper.getGoogleMapsKey()) - .map(tuple -> { - final String instanceId = tuple.getT1(); - final String googleMapsKey = tuple.getT2(); + Mono clientOrganizationMono = configService + .getInstanceId() + .flatMap(instanceId -> { final Organization organization = new Organization(); organization.setInstanceId(instanceId); organization.setAdminEmailDomainHash(adminEmailDomainHash); @@ -167,8 +167,6 @@ public Mono getOrganizationConfiguration(Mono dbOrga final OrganizationConfiguration config = new OrganizationConfiguration(); organization.setOrganizationConfiguration(config); - config.setGoogleMapsKey(googleMapsKey); - if (StringUtils.hasText(System.getenv("APPSMITH_OAUTH2_GOOGLE_CLIENT_ID"))) { config.addThirdPartyAuth("google"); } @@ -179,7 +177,14 @@ public Mono getOrganizationConfiguration(Mono dbOrga config.setIsFormLoginEnabled(!"true".equals(System.getenv("APPSMITH_FORM_LOGIN_DISABLED"))); - return organization; + return configService + .getInstanceVariables() + .map(instanceVariables -> instanceVariablesHelper.populateOrgConfigWithInstanceVariables( + instanceVariables, config)) + .map(organizationConfiguration -> { + organization.setOrganizationConfiguration(organizationConfiguration); + return organization; + }); }); return Mono.zip(dbOrganizationMono, clientOrganizationMono).flatMap(tuple -> { diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/OrganizationServiceCETest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/OrganizationServiceCETest.java index 8216da69839c..e6434a9b58e6 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/OrganizationServiceCETest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/OrganizationServiceCETest.java @@ -13,6 +13,7 @@ import com.appsmith.server.repositories.CacheableRepositoryHelper; import com.appsmith.server.repositories.OrganizationRepository; import com.appsmith.server.repositories.UserRepository; +import com.appsmith.server.services.ConfigService; import com.appsmith.server.services.FeatureFlagService; import com.appsmith.server.services.OrganizationService; import com.appsmith.server.solutions.EnvManager; @@ -81,6 +82,9 @@ class OrganizationServiceCETest { @MockBean FeatureFlagMigrationHelper featureFlagMigrationHelper; + @Autowired + ConfigService configService; + OrganizationConfiguration originalOrganizationConfiguration; @BeforeEach @@ -97,6 +101,7 @@ public void setup() throws IOException { null) .block(); + configService.updateInstanceVariables(new HashMap<>()).block(); // Make api_user super-user to test organization admin functionality // Todo change this to organization admin once we introduce multitenancy userRepository @@ -230,7 +235,7 @@ void setEmailVerificationEnabled_WithValidSMTPHost_Success() { StepVerifier.create(resultMono) .assertNext(organizationConfiguration -> { - assertThat(organizationConfiguration.isEmailVerificationEnabled()) + assertThat(organizationConfiguration.getEmailVerificationEnabled()) .isTrue(); }) .verifyComplete(); @@ -249,7 +254,7 @@ void setEmailVerificationEnabledFalseAndGetItBack() { StepVerifier.create(resultMono) .assertNext(organizationConfiguration -> { - assertThat(organizationConfiguration.isEmailVerificationEnabled()) + assertThat(organizationConfiguration.getEmailVerificationEnabled()) .isFalse(); }) .verifyComplete();