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

chore: Skip unnecessary RTS calls to optimise performance (#37949)

Browse Source 
## Description
Motivation came from this
[PoC](https://www.notion.so/appsmith/Evaluating-Performance-Bottlenecks-in-Pull-Based-Upgrades-Challenges-with-New-Relic-Telemetry-14bfe271b0e28063b9fdc08515ab3014)


Fixes https://github.com/appsmithorg/appsmith/issues/37948

## Automation

/ok-to-test tags="@tag.All"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/12181005719>
> Commit: d1f53e20d460cdf90bed3eef5f0552b7d871c197
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12181005719&attempt=2"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.All`
> Spec:
> <hr>Thu, 05 Dec 2024 15:10:54 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

## Summary by CodeRabbit

- **Bug Fixes**
- Improved error handling in the binding refactoring process to ensure
smoother operation.

- **Refactor**
- Enhanced efficiency of the binding refactoring method by adding a
conditional check to reduce unnecessary server calls.

- **Tests**
- Introduced a new test suite for the binding refactoring method,
validating various scenarios to ensure correct functionality.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
subratadeypappu 2024-12-05 21:14:23 +06:00 committed by GitHub
parent d03ad6ffe6
commit 6d41e77b1e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 163 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AstServiceCEImpl.java
View File

@ -144,6 +144,15 @@ public class AstServiceCEImpl implements AstServiceCE {
return Flux.fromIterable(bindingValues)
.flatMap(bindingValue -> {
if (!bindingValue.getValue().contains(oldName)) {
// This case is not handled in RTS either, so skipping the RTS call here will not affect the
// behavior.
// Example:
// - Old name: foo.bar
// - New name: foo.baz
// - Binding: "foo['bar']"
return Mono.just(Tuples.of(bindingValue, bindingValue.getValue()));
}
EntityRefactorRequest entityRefactorRequest = new EntityRefactorRequest(
bindingValue.getValue(), oldName, newName, evalVersion, isJSObject);
return rtsCaller

154
app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ASTServiceCETest.java Normal file
View File

@ -0,0 +1,154 @@
package com.appsmith.server.services.ce;
import com.appsmith.external.models.MustacheBindingToken;
import com.appsmith.server.services.AstService;
import lombok.extern.slf4j.Slf4j;
import org.junit.jupiter.api.Test;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.mock.mockito.SpyBean;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
import java.util.Collections;
import java.util.Map;
import java.util.Set;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.doReturn;
@SpringBootTest
@Slf4j
public class ASTServiceCETest {
@SpyBean
AstService astService;
@Test
void refactorNameInDynamicBindings_nullOrEmptyBindings_returnsEmptyMono() {
Mono<Map<MustacheBindingToken, String>> result =
astService.refactorNameInDynamicBindings(null, "abc", "xyz", 2, false);
StepVerifier.create(result).verifyComplete();
result = astService.refactorNameInDynamicBindings(Collections.emptySet(), "abc", "xyz", 2, false);
StepVerifier.create(result).verifyComplete();
}
@Test
void refactorNameInDynamicBindings_bindingWithoutOldName_returnsUnchangedMap() {
MustacheBindingToken token1 = new MustacheBindingToken();
token1.setValue("foo.bar");
MustacheBindingToken token2 = new MustacheBindingToken();
token2.setValue("baz.qux");
Set<MustacheBindingToken> bindings = Set.of(token1, token2);
Mono<Map<MustacheBindingToken, String>> result =
astService.refactorNameInDynamicBindings(bindings, "abc", "xyz", 2, false);
StepVerifier.create(result)
.assertNext(map -> {
assertThat(map).hasSize(2);
assertThat(map.get(token1)).isEqualTo("foo.bar");
assertThat(map.get(token2)).isEqualTo("baz.qux");
})
.verifyComplete();
}
@Test
void refactorNameInDynamicBindings_validBindings_returnsUpdatedBindings() {
MustacheBindingToken token1 = new MustacheBindingToken();
token1.setValue("abc['foo']");
MustacheBindingToken token2 = new MustacheBindingToken();
token2.setValue("xyz['bar']");
Set<MustacheBindingToken> bindings = Set.of(token1, token2);
String refactoredScript1 = "xyz['foo']";
String refactoredScript2 = "xyz['bar']";
Map<MustacheBindingToken, String> responseMap1 = Map.of(
token1, refactoredScript1,
token2, refactoredScript2);
doReturn(Mono.just(responseMap1))
.when(astService)
.refactorNameInDynamicBindings(Set.of(token1, token2), "abc", "xyz", 2, false);
Mono<Map<MustacheBindingToken, String>> result =
astService.refactorNameInDynamicBindings(bindings, "abc", "xyz", 2, false);
StepVerifier.create(result)
.assertNext(map -> {
assertThat(map).hasSize(2); // Only one binding refactored
assertThat(map.get(token1)).isEqualTo(refactoredScript1);
assertThat(map.get(token2)).isEqualTo(refactoredScript2);
})
.verifyComplete();
}
@Test
void refactorNameInDynamicBindings_whenValidJSObjectRequest_thenReturnUpdatedScript() {
MustacheBindingToken token = new MustacheBindingToken();
token.setValue("export default { myFun1() { Api1.run(); return Api1.data;}}");
Set<MustacheBindingToken> bindingValues = Set.of(token);
String oldName = "Api1";
String newName = "GetUsers";
int evalVersion = 2;
boolean isJSObject = true;
String refactoredScript = "export default { myFun1() { GetUsers.run(); return GetUsers.data;}}";
Map<MustacheBindingToken, String> responseMap = Map.of(token, refactoredScript);
doReturn(Mono.just(responseMap))
.when(astService)
.refactorNameInDynamicBindings(bindingValues, oldName, newName, evalVersion, isJSObject);
Mono<Map<MustacheBindingToken, String>> resultMono =
astService.refactorNameInDynamicBindings(bindingValues, oldName, newName, evalVersion, isJSObject);
StepVerifier.create(resultMono)
.assertNext(result -> {
assertThat(result).hasSize(1);
MustacheBindingToken key = bindingValues.iterator().next();
assertThat(result.containsKey(key)).isTrue();
assertThat(result.get(key)).isEqualTo(refactoredScript);
})
.verifyComplete();
}
@Test
void refactorNameInDynamicBindings_whenNoMatchingOldNameInJSObject_thenReturnOriginalScript() {
MustacheBindingToken token = new MustacheBindingToken();
token.setValue("export default { myFun1() { GetUsers.run(); return GetUsers.data;}}");
Set<MustacheBindingToken> bindingValues = Set.of(token);
String oldName = "Api1"; // oldName is not present in the script
String newName = "GetUsers";
int evalVersion = 2;
boolean isJSObject = true;
String refactoredScript = "export default { myFun1() { GetUsers.run(); return GetUsers.data;}}";
Map<MustacheBindingToken, String> responseMap = Map.of(token, refactoredScript);
doReturn(Mono.just(responseMap))
.when(astService)
.refactorNameInDynamicBindings(bindingValues, oldName, newName, evalVersion, isJSObject);
Mono<Map<MustacheBindingToken, String>> resultMono =
astService.refactorNameInDynamicBindings(bindingValues, oldName, newName, evalVersion, isJSObject);
StepVerifier.create(resultMono)
.assertNext(result -> {
assertThat(result).hasSize(1);
MustacheBindingToken key = bindingValues.iterator().next();
assertThat(result.containsKey(key)).isTrue();
assertThat(result.get(key)).isEqualTo(token.getValue()); // Script remains unchanged
})
.verifyComplete();
}
}