#294: Ensure parent directory exists and skip reading if file not exi…

…sts (#568)
devonfw · Sep 3, 2024 · 740505b · 740505b
1 parent 43e9252
commit 740505b
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 29 deletions.
diff --git a/cli/src/main/java/com/devonfw/tools/ide/environment/EnvironmentVariablesPropertiesFile.java b/cli/src/main/java/com/devonfw/tools/ide/environment/EnvironmentVariablesPropertiesFile.java
@@ -99,32 +99,34 @@ public void save() {
     }
     Path newPropertiesFilePath = this.propertiesFilePath;
     String propertiesFileName = this.propertiesFilePath.getFileName().toString();
+    Path propertiesParentPath = newPropertiesFilePath.getParent();
+
     if (LEGACY_PROPERTIES.equals(propertiesFileName)) {
-      newPropertiesFilePath = this.propertiesFilePath.getParent().resolve(DEFAULT_PROPERTIES);
+      newPropertiesFilePath = propertiesParentPath.resolve(DEFAULT_PROPERTIES);
       this.context.info("Converting legacy properties to {}", newPropertiesFilePath);
     }
+
+    this.context.getFileAccess().mkdirs(propertiesParentPath);
     List<VariableLine> lines = new ArrayList<>();
-    // TODO FixMe: we should simply skip reading if not exists instead of creating an empty file and then reading it. 
-    //             Also we should do mkdirs on parent folder to ensure it exists - see Review from https://github.com/devonfw/IDEasy/pull/374
-    if (!Files.exists(newPropertiesFilePath)) {
-      try {
-        Files.createFile(newPropertiesFilePath);
+
+    // Skip reading if the file does not exist
+    if (Files.exists(this.propertiesFilePath)) {
+      try (BufferedReader reader = Files.newBufferedReader(this.propertiesFilePath)) {
+        String line;
+        do {
+          line = reader.readLine();
+          if (line != null) {
+            VariableLine variableLine = VariableLine.of(line, this.context, getSource());
+            lines.add(variableLine);
+          }
+        } while (line != null);
       } catch (IOException e) {
-        throw new IllegalStateException("Failed to create properties file with" + newPropertiesFilePath, e);
+        throw new IllegalStateException("Failed to load existing properties from " + this.propertiesFilePath, e);
       }
+    } else {
+      this.context.debug("Properties file {} does not exist, skipping read.", newPropertiesFilePath);
     }
-    try (BufferedReader reader = Files.newBufferedReader(newPropertiesFilePath)) {
-      String line;
-      do {
-        line = reader.readLine();
-        if (line != null) {
-          VariableLine variableLine = VariableLine.of(line, this.context, getSource());
-          lines.add(variableLine);
-        }
-      } while (line != null);
-    } catch (IOException e) {
-      throw new IllegalStateException("Failed to load existing properties from " + this.propertiesFilePath, e);
-    }
+
     try (BufferedWriter writer = Files.newBufferedWriter(newPropertiesFilePath, StandardOpenOption.CREATE,
         StandardOpenOption.TRUNCATE_EXISTING)) {
       // copy and modify original lines from properties file

diff --git a/...c/test/java/com/devonfw/tools/ide/environment/EnvironmentVariablesPropertiesFileTest.java b/...c/test/java/com/devonfw/tools/ide/environment/EnvironmentVariablesPropertiesFileTest.java
@@ -21,18 +21,20 @@ class EnvironmentVariablesPropertiesFileTest extends Assertions {
   /**
    * Test of {@link EnvironmentVariablesPropertiesFile} including legacy support.
    */
+
+  private static final Path ENV_VAR_PATH = Path.of("src/test/resources/com/devonfw/tools/ide/env/var/");
+  private static final EnvironmentVariablesType TYPE = EnvironmentVariablesType.SETTINGS;
+
   @Test
   public void testLoad() {
 
     // arrange
-    AbstractEnvironmentVariables parent = null;
-    Path propertiesFilePath = Path.of("src/test/resources/com/devonfw/tools/ide/env/var/devon.properties");
-    EnvironmentVariablesType type = EnvironmentVariablesType.SETTINGS;
+    Path propertiesFilePath = ENV_VAR_PATH.resolve("devon.properties");
     // act
-    EnvironmentVariablesPropertiesFile variables = new EnvironmentVariablesPropertiesFile(parent, type,
+    EnvironmentVariablesPropertiesFile variables = new EnvironmentVariablesPropertiesFile(null, TYPE,
         propertiesFilePath, IdeTestContextMock.get());
     // assert
-    assertThat(variables.getType()).isSameAs(type);
+    assertThat(variables.getType()).isSameAs(TYPE);
     assertThat(variables.get("MVN_VERSION")).isEqualTo("3.9.0");
     assertThat(variables.get("IDE_TOOLS")).isEqualTo("mvn, npm");
     assertThat(variables.get("CREATE_START_SCRIPTS")).isEqualTo("eclipse");
@@ -67,10 +69,7 @@ void testSave(@TempDir Path tempDir) throws Exception {
     List<String> lines = Files.readAllLines(propertiesFilePath);
     assertThat(lines).containsExactlyElementsOf(linesToWrite);
 
-    AbstractEnvironmentVariables parent = null;
-    EnvironmentVariablesType type = EnvironmentVariablesType.SETTINGS;
-
-    EnvironmentVariablesPropertiesFile variables = new EnvironmentVariablesPropertiesFile(parent, type,
+    EnvironmentVariablesPropertiesFile variables = new EnvironmentVariablesPropertiesFile(null, TYPE,
         propertiesFilePath, IdeTestContextMock.get());
 
     // act
@@ -109,4 +108,28 @@ void testSave(@TempDir Path tempDir) throws Exception {
     lines = Files.readAllLines(propertiesFilePath);
     assertThat(lines).containsExactlyElementsOf(linesAfterSave);
   }
-}
+
+  @Test
+  void testSaveWithMissingParentFilePath() throws Exception {
+    // arrange
+    Path propertiesFilePath = ENV_VAR_PATH.resolve("test.properties");
+
+    EnvironmentVariablesPropertiesFile variables = new EnvironmentVariablesPropertiesFile(null, TYPE,
+        propertiesFilePath, IdeTestContextMock.get());
+
+    // act
+    variables.set("var1", "1.0", false);
+    variables.set("var2", "2", true);
+
+    variables.save();
+
+    // assert
+    List<String> linesAfterSave = new ArrayList<>();
+    linesAfterSave.add("export var2=2");
+    linesAfterSave.add("var1=1.0");
+
+    List<String> lines = Files.readAllLines(propertiesFilePath);
+    assertThat(lines).containsExactlyElementsOf(linesAfterSave);
+  }
+
+}