Skip to content

Commit

Permalink
chore: Update env variable used for update persistent DB URL (#35526)
Browse files Browse the repository at this point in the history
## Description
PR to fix the issue with DB url update from admin settings page. When we
migrated the env variable to `APPSMITH_DB_URL` instead of
`APPSMITH_MONGODB_URI` this functionality broke. We had a discussion to
remove this update functionality as this does not work in all the
setups. But as QA has already started pointing this as an issue raising
this PR.

## Automation

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

### 🔍 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/10297374384>
> Commit: dbef20c
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10297374384&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Settings`
> Spec:
> <hr>Thu, 08 Aug 2024 07:13:51 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


- **New Features**
- Updated the database connection string naming from "MongoDB URI" to
"Appsmith DB URL" in the Admin Settings, enhancing clarity in
configuration.
- Revised subtext in Admin Settings for improved user understanding of
the database URL's purpose.
  
- **Bug Fixes**
- Adjusted test cases to reflect the new naming convention, ensuring
consistent references to "APPSMITH_DB_URL" across various tests.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
abhvsn authored Aug 8, 2024
1 parent f0bccc6 commit 061e376
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 37 deletions.
4 changes: 2 additions & 2 deletions app/client/src/pages/AdminSettings/config/advanced.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ export const config: AdminConfigType = {
category: SettingCategories.ADVANCED,
controlType: SettingTypes.TEXTINPUT,
controlSubType: SettingSubtype.TEXT,
label: "MongoDB URI",
label: "Appsmith DB URL",
subText:
"* Appsmith internally uses MongoDB. Change to an external MongoDB for clustering",
"* Persistence database URL for Appsmith instance. Change this to an external database for clustering",
},
{
id: "APPSMITH_REDIS_URL",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

public enum EnvVariables {
APPSMITH_INSTANCE_NAME,
APPSMITH_MONGODB_URI,
APPSMITH_DB_URL,
APPSMITH_REDIS_URL,
APPSMITH_MAIL_ENABLED,
APPSMITH_MAIL_FROM,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,80 +116,76 @@ public void setup() {
@Test
public void simpleSample() {
final String content =
"APPSMITH_MONGODB_URI='first value'\nAPPSMITH_REDIS_URL='second value'\n\nAPPSMITH_INSTANCE_NAME='third value'";
"APPSMITH_DB_URL='first value'\nAPPSMITH_REDIS_URL='second value'\n\nAPPSMITH_INSTANCE_NAME='third value'";

assertThat(envManager.transformEnvContent(content, Map.of("APPSMITH_MONGODB_URI", "new first value")))
assertThat(envManager.transformEnvContent(content, Map.of("APPSMITH_DB_URL", "new first value")))
.containsExactly(
"APPSMITH_MONGODB_URI='new first value'",
"APPSMITH_DB_URL='new first value'",
"APPSMITH_REDIS_URL='second value'",
"",
"APPSMITH_INSTANCE_NAME='third value'");

assertThat(envManager.transformEnvContent(content, Map.of("APPSMITH_REDIS_URL", "new second value")))
.containsExactly(
"APPSMITH_MONGODB_URI='first value'",
"APPSMITH_DB_URL='first value'",
"APPSMITH_REDIS_URL='new second value'",
"",
"APPSMITH_INSTANCE_NAME='third value'");

assertThat(envManager.transformEnvContent(content, Map.of("APPSMITH_INSTANCE_NAME", "new third value")))
.containsExactly(
"APPSMITH_MONGODB_URI='first value'",
"APPSMITH_DB_URL='first value'",
"APPSMITH_REDIS_URL='second value'",
"",
"APPSMITH_INSTANCE_NAME='new third value'");

assertThat(envManager.transformEnvContent(
content,
Map.of(
"APPSMITH_MONGODB_URI", "new first value",
"APPSMITH_DB_URL", "new first value",
"APPSMITH_INSTANCE_NAME", "new third value")))
.containsExactly(
"APPSMITH_MONGODB_URI='new first value'",
"APPSMITH_DB_URL='new first value'",
"APPSMITH_REDIS_URL='second value'",
"",
"APPSMITH_INSTANCE_NAME='new third value'");
}

@Test
public void emptyValues() {
final String content =
"APPSMITH_MONGODB_URI=first value\nAPPSMITH_REDIS_URL=\n\nAPPSMITH_INSTANCE_NAME=third value";
final String content = "APPSMITH_DB_URL=first value\nAPPSMITH_REDIS_URL=\n\nAPPSMITH_INSTANCE_NAME=third value";

assertThat(envManager.transformEnvContent(content, Map.of("APPSMITH_REDIS_URL", "new second value")))
.containsExactly(
"APPSMITH_MONGODB_URI=first value",
"APPSMITH_DB_URL=first value",
"APPSMITH_REDIS_URL='new second value'",
"",
"APPSMITH_INSTANCE_NAME=third value");

assertThat(envManager.transformEnvContent(content, Map.of("APPSMITH_REDIS_URL", "")))
.containsExactly(
"APPSMITH_MONGODB_URI=first value",
"APPSMITH_REDIS_URL=",
"",
"APPSMITH_INSTANCE_NAME=third value");
"APPSMITH_DB_URL=first value", "APPSMITH_REDIS_URL=", "", "APPSMITH_INSTANCE_NAME=third value");
}

@Test
public void quotedValues() {
final String content =
"APPSMITH_MONGODB_URI='first value'\nAPPSMITH_REDIS_URL=\"quoted value\"\n\nAPPSMITH_INSTANCE_NAME='third value'";
"APPSMITH_DB_URL='first value'\nAPPSMITH_REDIS_URL=\"quoted value\"\n\nAPPSMITH_INSTANCE_NAME='third value'";

assertThat(envManager.transformEnvContent(
content,
Map.of(
"APPSMITH_MONGODB_URI", "new first value",
"APPSMITH_DB_URL", "new first value",
"APPSMITH_REDIS_URL", "new second value")))
.containsExactly(
"APPSMITH_MONGODB_URI='new first value'",
"APPSMITH_DB_URL='new first value'",
"APPSMITH_REDIS_URL='new second value'",
"",
"APPSMITH_INSTANCE_NAME='third value'");

assertThat(envManager.transformEnvContent(content, Map.of("APPSMITH_REDIS_URL", "")))
.containsExactly(
"APPSMITH_MONGODB_URI='first value'",
"APPSMITH_DB_URL='first value'",
"APPSMITH_REDIS_URL=",
"",
"APPSMITH_INSTANCE_NAME='third value'");
Expand All @@ -200,7 +196,7 @@ public void quotedValues() {
"APPSMITH_INSTANCE_NAME", "Sponge-bob's Instance",
"APPSMITH_REDIS_URL", "value with \" char in it")))
.containsExactly(
"APPSMITH_MONGODB_URI='first value'",
"APPSMITH_DB_URL='first value'",
"APPSMITH_REDIS_URL='value with \" char in it'",
"",
"APPSMITH_INSTANCE_NAME='Sponge-bob'\"'\"'s Instance'");
Expand All @@ -209,11 +205,10 @@ public void quotedValues() {
@Test
public void parseEmptyValues() {

assertThat(
envManager.parseToMap(
"APPSMITH_MONGODB_URI='first value'\nAPPSMITH_REDIS_URL=\n\nAPPSMITH_INSTANCE_NAME='third value'"))
assertThat(envManager.parseToMap(
"APPSMITH_DB_URL='first value'\nAPPSMITH_REDIS_URL=\n\nAPPSMITH_INSTANCE_NAME='third value'"))
.containsExactlyInAnyOrderEntriesOf(Map.of(
"APPSMITH_MONGODB_URI", "first value",
"APPSMITH_DB_URL", "first value",
"APPSMITH_REDIS_URL", "",
"APPSMITH_INSTANCE_NAME", "third value"));
}
Expand All @@ -223,9 +218,9 @@ public void parseQuotedValues() {

assertThat(
envManager.parseToMap(
"APPSMITH_MONGODB_URI=first\nAPPSMITH_REDIS_URL=\"quoted value\"\n\nAPPSMITH_INSTANCE_NAME='third value'"))
"APPSMITH_DB_URL=first\nAPPSMITH_REDIS_URL=\"quoted value\"\n\nAPPSMITH_INSTANCE_NAME='third value'"))
.containsExactlyInAnyOrderEntriesOf(Map.of(
"APPSMITH_MONGODB_URI", "first",
"APPSMITH_DB_URL", "first",
"APPSMITH_REDIS_URL", "quoted value",
"APPSMITH_INSTANCE_NAME", "third value"));

Expand All @@ -245,12 +240,12 @@ public void parseTestWithEscapes() {
@Test
public void disallowedVariable() {
final String content =
"APPSMITH_MONGODB_URI=first value\nDISALLOWED_NASTY_STUFF=\"quoted value\"\n\nAPPSMITH_INSTANCE_NAME=third value";
"APPSMITH_DB_URL=first value\nDISALLOWED_NASTY_STUFF=\"quoted value\"\n\nAPPSMITH_INSTANCE_NAME=third value";

assertThatThrownBy(() -> envManager.transformEnvContent(
content,
Map.of(
"APPSMITH_MONGODB_URI", "new first value",
"APPSMITH_DB_URL", "new first value",
"DISALLOWED_NASTY_STUFF", "new second value")))
.matches(value -> value instanceof AppsmithException
&& AppsmithError.GENERIC_BAD_REQUEST.equals(((AppsmithException) value).getError()));
Expand All @@ -259,15 +254,15 @@ public void disallowedVariable() {
@Test
public void addNewVariable() {
final String content =
"APPSMITH_MONGODB_URI='first value'\nAPPSMITH_REDIS_URL='quoted value'\n\nAPPSMITH_INSTANCE_NAME='third value'";
"APPSMITH_DB_URL='first value'\nAPPSMITH_REDIS_URL='quoted value'\n\nAPPSMITH_INSTANCE_NAME='third value'";

assertThat(envManager.transformEnvContent(
content,
Map.of(
"APPSMITH_MONGODB_URI", "new first value",
"APPSMITH_DB_URL", "new first value",
"APPSMITH_DISABLE_TELEMETRY", "false")))
.containsExactly(
"APPSMITH_MONGODB_URI='new first value'",
"APPSMITH_DB_URL='new first value'",
"APPSMITH_REDIS_URL='quoted value'",
"",
"APPSMITH_INSTANCE_NAME='third value'",
Expand All @@ -277,15 +272,15 @@ public void addNewVariable() {
@Test
public void setValueWithQuotes() {
final String content =
"APPSMITH_MONGODB_URI='first value'\nAPPSMITH_REDIS_URL='quoted value'\n\nAPPSMITH_INSTANCE_NAME='third value'";
"APPSMITH_DB_URL='first value'\nAPPSMITH_REDIS_URL='quoted value'\n\nAPPSMITH_INSTANCE_NAME='third value'";

assertThat(envManager.transformEnvContent(
content,
Map.of(
"APPSMITH_MONGODB_URI", "'just quotes'",
"APPSMITH_DB_URL", "'just quotes'",
"APPSMITH_DISABLE_TELEMETRY", "some quotes 'inside' it")))
.containsExactly(
"APPSMITH_MONGODB_URI=\"'\"'just quotes'\"'\"",
"APPSMITH_DB_URL=\"'\"'just quotes'\"'\"",
"APPSMITH_REDIS_URL='quoted value'",
"",
"APPSMITH_INSTANCE_NAME='third value'",
Expand All @@ -310,7 +305,7 @@ public void setEnv_AndGetAll() {
EnvManager envManagerInner = Mockito.mock(EnvManagerImpl.class);

Map<String, String> envs = new HashMap<>();
envs.put("APPSMITH_MONGODB_URI", "mongo-url");
envs.put("APPSMITH_DB_URL", "mongo-url");
envs.put("APPSMITH_DISABLE_TELEMETRY", "");

Mockito.when(envManagerInner.getAll()).thenReturn(Mono.just(envs));
Expand Down

0 comments on commit 061e376

Please sign in to comment.