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

chore: Remove unused, misleading response to PUT /env APIs (#26431)

Browse Source 
This response object to `PUT /env` API appears to indicate whether
restart is required or not, because it includes the field
`isRestartRequired`, but this is actually not used at all on the client.
We might've used it in the past, but we don't anymore. This is a
misleading response DTO, and it's already wasted my time thrice! 😭
This commit is contained in:
Shrikant Sharat Kandula 2023-08-18 15:59:06 +05:30 committed by GitHub
parent 9e5a38b3d9
commit d26ae16bca
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 15 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/InstanceAdminControllerCE.java
View File

@ -2,7 +2,6 @@ package com.appsmith.server.controllers.ce;
import com.appsmith.external.views.Views; import com.appsmith.external.views.Views;
import com.appsmith.server.constants.Url; import com.appsmith.server.constants.Url;
import com.appsmith.server.dtos.EnvChangesResponseDTO;
import com.appsmith.server.dtos.ResponseDTO; import com.appsmith.server.dtos.ResponseDTO;
import com.appsmith.server.dtos.TestEmailConfigRequestDTO; import com.appsmith.server.dtos.TestEmailConfigRequestDTO;
import com.appsmith.server.solutions.EnvManager; import com.appsmith.server.solutions.EnvManager;
@ -48,21 +47,20 @@ public class InstanceAdminControllerCE {
@PutMapping( @PutMapping(
value = "/env", value = "/env",
consumes = {MediaType.APPLICATION_JSON_VALUE}) consumes = {MediaType.APPLICATION_JSON_VALUE})
public Mono<ResponseDTO<EnvChangesResponseDTO>> saveEnvChangesJSON( public Mono<ResponseDTO<Void>> saveEnvChangesJSON(@Valid @RequestBody Map<String, String> changes) {
@Valid @RequestBody Map<String, String> changes) {
log.debug("Applying env updates {}", changes.keySet()); log.debug("Applying env updates {}", changes.keySet());
return envManager.applyChanges(changes).map(res -> new ResponseDTO<>(HttpStatus.OK.value(), res, null)); return envManager.applyChanges(changes).thenReturn(new ResponseDTO<>(HttpStatus.OK.value(), null, null));
} }
@JsonView(Views.Public.class) @JsonView(Views.Public.class)
@PutMapping( @PutMapping(
value = "/env", value = "/env",
consumes = {MediaType.MULTIPART_FORM_DATA_VALUE}) consumes = {MediaType.MULTIPART_FORM_DATA_VALUE})
public Mono<ResponseDTO<EnvChangesResponseDTO>> saveEnvChangesMultipartFormData(ServerWebExchange exchange) { public Mono<ResponseDTO<Void>> saveEnvChangesMultipartFormData(ServerWebExchange exchange) {
log.debug("Applying env updates from form data"); log.debug("Applying env updates from form data");
return exchange.getMultipartData() return exchange.getMultipartData()
.flatMap(envManager::applyChangesFromMultipartFormData) .flatMap(envManager::applyChangesFromMultipartFormData)
.map(res -> new ResponseDTO<>(HttpStatus.OK.value(), res, null)); .thenReturn(new ResponseDTO<>(HttpStatus.OK.value(), null, null));
} }
@JsonView(Views.Public.class) @JsonView(Views.Public.class)

13
app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/EnvChangesResponseDTO.java
View File

@ -1,13 +0,0 @@
package com.appsmith.server.dtos;
import com.fasterxml.jackson.annotation.JsonProperty;
import lombok.AllArgsConstructor;
import lombok.Data;
@Data
@AllArgsConstructor
public class EnvChangesResponseDTO {
@JsonProperty(value = "isRestartRequired")
boolean isRestartRequired;
}

5
app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCE.java
View File

@ -1,7 +1,6 @@
package com.appsmith.server.solutions.ce; package com.appsmith.server.solutions.ce;
import com.appsmith.server.domains.User; import com.appsmith.server.domains.User;
import com.appsmith.server.dtos.EnvChangesResponseDTO;
import com.appsmith.server.dtos.TestEmailConfigRequestDTO; import com.appsmith.server.dtos.TestEmailConfigRequestDTO;
import org.springframework.http.codec.multipart.Part; import org.springframework.http.codec.multipart.Part;
import org.springframework.util.MultiValueMap; import org.springframework.util.MultiValueMap;
@ -15,9 +14,9 @@ public interface EnvManagerCE {
List<String> transformEnvContent(String envContent, Map<String, String> changes); List<String> transformEnvContent(String envContent, Map<String, String> changes);
Mono<EnvChangesResponseDTO> applyChanges(Map<String, String> changes); Mono<Void> applyChanges(Map<String, String> changes);
Mono<EnvChangesResponseDTO> applyChangesFromMultipartFormData(MultiValueMap<String, Part> formData); Mono<Void> applyChangesFromMultipartFormData(MultiValueMap<String, Part> formData);
void setAnalyticsEventAction( void setAnalyticsEventAction(
Map<String, Object> properties, String newVariable, String originalVariable, String authEnv); Map<String, Object> properties, String newVariable, String originalVariable, String authEnv);

10
app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java
View File

@ -9,7 +9,6 @@ import com.appsmith.server.constants.FieldName;
import com.appsmith.server.domains.Tenant; import com.appsmith.server.domains.Tenant;
import com.appsmith.server.domains.TenantConfiguration; import com.appsmith.server.domains.TenantConfiguration;
import com.appsmith.server.domains.User; import com.appsmith.server.domains.User;
import com.appsmith.server.dtos.EnvChangesResponseDTO;
import com.appsmith.server.dtos.TestEmailConfigRequestDTO; import com.appsmith.server.dtos.TestEmailConfigRequestDTO;
import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.exceptions.AppsmithException;
@ -336,7 +335,7 @@ public class EnvManagerCEImpl implements EnvManagerCE {
} }
@Override @Override
public Mono<EnvChangesResponseDTO> applyChanges(Map<String, String> changes) { public Mono<Void> applyChanges(Map<String, String> changes) {
// This flow is pertinent for any variables that need to change in the .env file or be saved in the tenant // This flow is pertinent for any variables that need to change in the .env file or be saved in the tenant
// configuration // configuration
return verifyCurrentUserIsSuper() return verifyCurrentUserIsSuper()
@ -417,8 +416,7 @@ public class EnvManagerCEImpl implements EnvManagerCE {
emailConfig.setEmailEnabled("true".equals(changesCopy.remove(APPSMITH_MAIL_SMTP_AUTH.name()))); emailConfig.setEmailEnabled("true".equals(changesCopy.remove(APPSMITH_MAIL_SMTP_AUTH.name())));
} }
if (javaMailSender instanceof JavaMailSenderImpl) { if (javaMailSender instanceof JavaMailSenderImpl javaMailSenderImpl) {
JavaMailSenderImpl javaMailSenderImpl = (JavaMailSenderImpl) javaMailSender;
if (changesCopy.containsKey(APPSMITH_MAIL_HOST.name())) { if (changesCopy.containsKey(APPSMITH_MAIL_HOST.name())) {
javaMailSenderImpl.setHost(changesCopy.remove(APPSMITH_MAIL_HOST.name())); javaMailSenderImpl.setHost(changesCopy.remove(APPSMITH_MAIL_HOST.name()));
} }
@ -446,12 +444,12 @@ public class EnvManagerCEImpl implements EnvManagerCE {
"true".equals(changesCopy.remove(APPSMITH_DISABLE_TELEMETRY.name()))); "true".equals(changesCopy.remove(APPSMITH_DISABLE_TELEMETRY.name())));
} }
return dependentTasks.thenReturn(new EnvChangesResponseDTO(true)); return dependentTasks.then();
}); });
} }
@Override @Override
public Mono<EnvChangesResponseDTO> applyChangesFromMultipartFormData(MultiValueMap<String, Part> formData) { public Mono<Void> applyChangesFromMultipartFormData(MultiValueMap<String, Part> formData) {
return Flux.fromIterable(formData.entrySet()) return Flux.fromIterable(formData.entrySet())
.flatMap(entry -> { .flatMap(entry -> {
final String key = entry.getKey(); final String key = entry.getKey();

8
app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/UserSignupCEImpl.java
View File

@ -8,7 +8,6 @@ import com.appsmith.server.domains.LoginSource;
import com.appsmith.server.domains.User; import com.appsmith.server.domains.User;
import com.appsmith.server.domains.UserData; import com.appsmith.server.domains.UserData;
import com.appsmith.server.domains.UserState; import com.appsmith.server.domains.UserState;
import com.appsmith.server.dtos.EnvChangesResponseDTO;
import com.appsmith.server.dtos.UserSignupDTO; import com.appsmith.server.dtos.UserSignupDTO;
import com.appsmith.server.dtos.UserSignupRequestDTO; import com.appsmith.server.dtos.UserSignupRequestDTO;
import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithError;
@ -279,17 +278,20 @@ public class UserSignupCEImpl implements UserSignupCE {
return pair.getT2(); return pair.getT2();
}); });
Mono<EnvChangesResponseDTO> applyEnvManagerChangesMono = envManager Mono<Void> applyEnvManagerChangesMono = envManager
.applyChanges(Map.of( .applyChanges(Map.of(
APPSMITH_DISABLE_TELEMETRY.name(), APPSMITH_DISABLE_TELEMETRY.name(),
String.valueOf(!userFromRequest.isAllowCollectingAnonymousData()), String.valueOf(!userFromRequest.isAllowCollectingAnonymousData()),
APPSMITH_ADMIN_EMAILS.name(), APPSMITH_ADMIN_EMAILS.name(),
user.getEmail())) user.getEmail()))
// We need a non-empty value for `.elapsed` to work.
.thenReturn(true)
.elapsed() .elapsed()
.map(pair -> { .map(pair -> {
log.debug("UserSignupCEImpl::Time taken to apply env changes: {} ms", pair.getT1()); log.debug("UserSignupCEImpl::Time taken to apply env changes: {} ms", pair.getT1());
return pair.getT2(); return pair.getT2();
}); })
.then();
/* /*
* Here, we have decided to move these 2 analytics events to a separate thread. * Here, we have decided to move these 2 analytics events to a separate thread.