From 740505b07ec25acf108ff21983efa64d5a7b6864 Mon Sep 17 00:00:00 2001 From: ndemirca <157051033+ndemirca@users.noreply.github.com> Date: Tue, 3 Sep 2024 16:50:57 +0200 Subject: [PATCH] #294: Ensure parent directory exists and skip reading if file not exists (#568) --- .../EnvironmentVariablesPropertiesFile.java | 40 +++++++++-------- ...nvironmentVariablesPropertiesFileTest.java | 43 ++++++++++++++----- 2 files changed, 54 insertions(+), 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 index f2a915200..f97f075da 100644 --- 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 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/cli/src/test/java/com/devonfw/tools/ide/environment/EnvironmentVariablesPropertiesFileTest.java b/cli/src/test/java/com/devonfw/tools/ide/environment/EnvironmentVariablesPropertiesFileTest.java index 926e7501a..065ff1c19 100644 --- a/cli/src/test/java/com/devonfw/tools/ide/environment/EnvironmentVariablesPropertiesFileTest.java +++ b/cli/src/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 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); } -} \ No newline at end of file + + @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 linesAfterSave = new ArrayList<>(); + linesAfterSave.add("export var2=2"); + linesAfterSave.add("var1=1.0"); + + List lines = Files.readAllLines(propertiesFilePath); + assertThat(lines).containsExactlyElementsOf(linesAfterSave); + } + +}