Fix: update tsconfig and log message (#18987)

* Fix: update tsconfig and log message

Instead of updating and throwing
exception for 'tsconfig.json'.
Generate .bak file,
update file and log info if the
file is updated.

Fixes #18984

* unify backup file for ts config/definition

both make a .bak file with a 'stamp'.
Only log for tsDefinition instead of throwing.

* Clean up commented code.

Only log warning one time

* Reset flag in tests

* Minor cleanup

* update message

* Update flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateTsDefinitions.java

---------

Co-authored-by: Mikhail Shabarov <61410877+mshabarov@users.noreply.github.com>

Author: caalador

flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateTsConfig.java

@@ -40,6 +40,12 @@
  */
 public class TaskGenerateTsConfig extends AbstractTaskClientGenerator {
 
+    /**
+     * Keeps track of whether a warning update has already been logged. This is
+     * used to avoid spamming the log with the same message.
+     */
+    protected static boolean warningEmitted = false;
+
     private static final String COMPILER_OPTIONS = "compilerOptions";
 
     static final String TSCONFIG_JSON = "tsconfig.json";
@@ -52,15 +58,17 @@ public class TaskGenerateTsConfig extends AbstractTaskClientGenerator {
             "v23.3.0", "v23.2", "v23.1", "v22", "v14", "osgi", "v23.3.4",
             "v23.3.4-hilla" };
 
-    //@formatter:off
-    static final String ERROR_MESSAGE =
-            "%n%n**************************************************************************"
-            + "%n*  TypeScript config file 'tsconfig.json' has been updated to the latest *"
-            + "%n*  version by Vaadin. Please verify that the updated 'tsconfig.json'     *"
-            + "%n*  file contains configuration needed for your project (add missing part *"
-            + "%n*  from the old file if necessary) and restart the application.          *"
-            + "%n**************************************************************************%n%n";
-    //@formatter:on
+    static final String ERROR_MESSAGE = """
+
+            **************************************************************************
+            *  TypeScript config file 'tsconfig.json' has been updated to the latest *
+            *  version by Vaadin. Please verify that the updated 'tsconfig.json'     *
+            *  file contains configuration needed for your project (add any missing  *
+            *  parts from the old file if necessary) and restart the application.    *
+            *  Old configuration is stored as a '.bak' file.                         *
+            **************************************************************************
+
+            """;
 
     private Options options;
 
@@ -216,11 +224,18 @@ private void overrideIfObsolete() throws ExecutionFailedException {
                 }
             }
 
+            File backupFile = File.createTempFile(
+                    projectTsConfigFile.getName() + ".", ".bak",
+                    projectTsConfigFile.getParentFile());
+            FileIOUtils.writeIfChanged(backupFile, projectTsConfigAsString);
             // Project's TS config has a custom content -
             // rewrite and throw an exception with explanations
             FileIOUtils.writeIfChanged(projectTsConfigFile,
                     latestTsConfigTemplate);
-            throw new ExecutionFailedException(String.format(ERROR_MESSAGE));
+            if (!warningEmitted) {
+                log().warn(ERROR_MESSAGE);
+                warningEmitted = true;
+            }
         } catch (IOException e) {
             throw new UncheckedIOException(e);
         }

flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateTsDefinitions.java

@@ -19,7 +19,6 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.file.Files;
-import java.nio.file.Path;
 import java.nio.file.StandardOpenOption;
 import java.util.regex.Pattern;
 
@@ -43,21 +42,18 @@ public class TaskGenerateTsDefinitions extends AbstractTaskClientGenerator {
     private static final String DECLARE_CSS_MODULE = "declare module '*.css?inline' {";
     static final String UPDATE_MESSAGE = """
 
-
             ***************************************************************************
             *  The TypeScript type declaration file 'types.d.ts' has been updated     *
             *  to the latest version by Vaadin. Previous content has been backed up   *
-            *  on 'types.d.ts.flowBackup' file. Please verify that the updated        *
-            *  'types.d.ts' file contains configuration needed for your project, and  *
-            *  then delete the backup file.                                           *
+            *  as a '.bak' file. Please verify that the updated 'types.d.ts' file    *
+            *  contains configuration needed for your project, and then delete the    *
+            *  backup file.                                                           *
             ***************************************************************************
 
-
             """;
 
     static final String CHECK_CONTENT_MESSAGE = """
 
-
             ****************************************************************************
             *  The TypeScript type declaration file 'types.d.ts' has been customized.  *
             *  Make sure the exact following configuration is present in that file:    *
@@ -68,8 +64,12 @@ public class TaskGenerateTsDefinitions extends AbstractTaskClientGenerator {
             *  and then update it with your custom contents.                           *
             ****************************************************************************"
 
-
             """;
+    /**
+     * Keeps track of whether a warning update has already been logged. This is
+     * used to avoid spamming the log with the same message.
+     */
+    protected static boolean warningEmitted = false;
 
     static final String TS_DEFINITIONS = "types.d.ts";
     static final Pattern COMMENT_LINE = Pattern.compile("(?m)^/[/*].*\\R");
@@ -148,11 +148,12 @@ private void updateIfContentMissing() throws ExecutionFailedException {
                         "Updating custom {} to add '*.css?inline' module declaration",
                         TS_DEFINITIONS);
                 UpdateMode updateMode = computeUpdateMode(content);
-                if (updateMode == UpdateMode.UPDATE_AND_THROW) {
+                if (updateMode == UpdateMode.UPDATE_AND_BACKUP) {
                     try {
-                        Path backupFile = tsDefinitions.toPath().getParent()
-                                .resolve(TS_DEFINITIONS + ".flowBackup");
-                        Files.copy(tsDefinitions.toPath(), backupFile);
+                        File backupFile = File.createTempFile(
+                                tsDefinitions.getName() + ".", ".bak",
+                                tsDefinitions.getParentFile());
+                        FileIOUtils.writeIfChanged(backupFile, content);
                         log().debug("Created {} backup copy on {}",
                                 TS_DEFINITIONS, backupFile);
                     } catch (IOException ex) {
@@ -171,8 +172,10 @@ private void updateIfContentMissing() throws ExecutionFailedException {
                         Files.writeString(tsDefinitions.toPath(),
                                 uncommentedDefaultContent,
                                 StandardOpenOption.APPEND);
-                        if (updateMode == UpdateMode.UPDATE_AND_THROW) {
-                            throw new ExecutionFailedException(UPDATE_MESSAGE);
+                        if (updateMode == UpdateMode.UPDATE_AND_BACKUP
+                                && !warningEmitted) {
+                            log().warn(UPDATE_MESSAGE);
+                            warningEmitted = true;
                         }
                     }
                 } catch (IOException ex) {
@@ -185,7 +188,7 @@ private void updateIfContentMissing() throws ExecutionFailedException {
     }
 
     private enum UpdateMode {
-        REPLACE, UPDATE, UPDATE_AND_THROW
+        REPLACE, UPDATE, UPDATE_AND_BACKUP
     }
 
     private UpdateMode computeUpdateMode(String content)
@@ -220,7 +223,7 @@ private UpdateMode computeUpdateMode(String content)
         // types.d.ts has been customized, but does not seem to contain content
         // required by flow. Append what is needed and throw an exception so
         // that developer can check the changes are ok
-        return UpdateMode.UPDATE_AND_THROW;
+        return UpdateMode.UPDATE_AND_BACKUP;
     }
 
     private static String removeComments(String content) {

flow-server/src/test/java/com/vaadin/flow/server/frontend/TaskGenerateTsConfigTest.java

@@ -15,6 +15,7 @@
  */
 package com.vaadin.flow.server.frontend;
 
+import static com.vaadin.flow.server.frontend.TaskGenerateTsConfig.ERROR_MESSAGE;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import java.io.File;
@@ -25,19 +26,22 @@
 import java.nio.file.Path;
 import java.util.Objects;
 
+import net.jcip.annotations.NotThreadSafe;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
+import org.mockito.MockedStatic;
 import org.mockito.Mockito;
 
 import com.vaadin.experimental.FeatureFlags;
 import com.vaadin.flow.di.Lookup;
 import com.vaadin.flow.server.ExecutionFailedException;
 
+@NotThreadSafe
 public class TaskGenerateTsConfigTest {
     static private String LATEST_VERSION = "9.1";
 
@@ -57,6 +61,7 @@ public void setUp() throws IOException {
                 .withFeatureFlags(featureFlags);
 
         taskGenerateTsConfig = new TaskGenerateTsConfig(options);
+        taskGenerateTsConfig.warningEmitted = false;
     }
 
     @Test
@@ -204,20 +209,43 @@ public void tsConfigHasLatestVersion_noUpdates()
     }
 
     @Test
-    public void tsConfigHasCustomCodes_updatesAndThrows() throws IOException {
+    public void tsConfigHasCustomCodes_updatesAndLogsWarning()
+            throws IOException, ExecutionFailedException {
         File tsconfig = writeTestTsConfigContent(
                 "tsconfig-custom-content.json");
-        try {
+        MockLogger logger = new MockLogger();
+        try (MockedStatic<AbstractTaskClientGenerator> client = Mockito
+                .mockStatic(AbstractTaskClientGenerator.class,
+                        Mockito.CALLS_REAL_METHODS)) {
+            client.when(() -> AbstractTaskClientGenerator.log())
+                    .thenReturn(logger);
+            taskGenerateTsConfig.execute();
+        }
+        String tsConfigString = FileUtils.readFileToString(tsconfig, UTF_8);
+        Assert.assertTrue(tsConfigString.contains(
+                "\"@vaadin/flow-frontend\": [\"generated/jar-resources\"],"));
+        Assert.assertTrue(logger.getLogs().contains(ERROR_MESSAGE));
+    }
+
+    @Test
+    public void warningIsLoggedOnlyOncePerRun()
+            throws IOException, ExecutionFailedException {
+        File tsconfig = writeTestTsConfigContent(
+                "tsconfig-custom-content.json");
+        MockLogger logger = new MockLogger();
+        try (MockedStatic<AbstractTaskClientGenerator> client = Mockito
+                .mockStatic(AbstractTaskClientGenerator.class,
+                        Mockito.CALLS_REAL_METHODS)) {
+            client.when(() -> AbstractTaskClientGenerator.log())
+                    .thenReturn(logger);
+            taskGenerateTsConfig.execute();
+            Assert.assertTrue(logger.getLogs().contains(ERROR_MESSAGE));
+            logger.clearLogs();
+            tsconfig.delete();
+            writeTestTsConfigContent("tsconfig-custom-content.json");
             taskGenerateTsConfig.execute();
-        } catch (Exception e) {
-            Assert.assertTrue(e.getMessage().contains(
-                    "TypeScript config file 'tsconfig.json' has been updated to the latest"));
-            String tsConfigString = FileUtils.readFileToString(tsconfig, UTF_8);
-            Assert.assertTrue(tsConfigString.contains(
-                    "\"@vaadin/flow-frontend\": [\"generated/jar-resources\"],"));
-            return;
+            Assert.assertFalse(logger.getLogs().contains(ERROR_MESSAGE));
         }
-        Assert.fail("Expected exception to be thrown");
     }
 
     @Test

flow-server/src/test/java/com/vaadin/flow/server/frontend/TaskGenerateTsDefinitionsTest.java

@@ -22,6 +22,7 @@
 import java.nio.file.Path;
 import java.nio.file.attribute.FileTime;
 
+import net.jcip.annotations.NotThreadSafe;
 import org.apache.commons.io.IOUtils;
 import org.hamcrest.CoreMatchers;
 import org.hamcrest.MatcherAssert;
@@ -30,6 +31,7 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
+import org.mockito.MockedStatic;
 import org.mockito.Mockito;
 
 import com.vaadin.flow.di.Lookup;
@@ -38,6 +40,7 @@
 import static com.vaadin.flow.server.frontend.TaskGenerateTsDefinitions.TS_DEFINITIONS;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
+@NotThreadSafe
 public class TaskGenerateTsDefinitionsTest {
     @Rule
     public TemporaryFolder temporaryFolder = new TemporaryFolder();
@@ -50,6 +53,7 @@ public void setUp() throws IOException {
         Options options = new Options(Mockito.mock(Lookup.class), outputFolder);
 
         taskGenerateTsDefinitions = new TaskGenerateTsDefinitions(options);
+        taskGenerateTsDefinitions.warningEmitted = false;
     }
 
     @Test
@@ -199,7 +203,7 @@ public void tsDefinition_oldHillaContents_ignoringSingleLineComments_tsDefinitio
     }
 
     @Test
-    public void customTsDefinition_missingFlowContents_tsDefinitionUpdatedAndExceptionThrown()
+    public void customTsDefinition_missingFlowContents_tsDefinitionUpdatedAndWarningLogged()
             throws Exception {
         Path typesTSfile = new File(outputFolder, "types.d.ts").toPath();
         String originalContent = """
@@ -211,11 +215,17 @@ public void customTsDefinition_missingFlowContents_tsDefinitionUpdatedAndExcepti
                 export type JTDForm = typeof jtdForms[number];
                 """;
         Files.writeString(typesTSfile, originalContent);
-        ExecutionFailedException exception = Assert.assertThrows(
-                ExecutionFailedException.class,
-                taskGenerateTsDefinitions::execute);
-        MatcherAssert.assertThat(exception.getMessage(), CoreMatchers
-                .containsString(TaskGenerateTsDefinitions.UPDATE_MESSAGE));
+
+        MockLogger logger = new MockLogger();
+        try (MockedStatic<AbstractTaskClientGenerator> client = Mockito
+                .mockStatic(AbstractTaskClientGenerator.class,
+                        Mockito.CALLS_REAL_METHODS)) {
+            client.when(() -> AbstractTaskClientGenerator.log())
+                    .thenReturn(logger);
+            taskGenerateTsDefinitions.execute();
+        }
+        Assert.assertTrue(logger.getLogs()
+                .contains(TaskGenerateTsDefinitions.UPDATE_MESSAGE));
         Assert.assertFalse(
                 "Should not generate types.d.ts when already existing",
                 taskGenerateTsDefinitions.shouldGenerate());
@@ -227,17 +237,24 @@ public void customTsDefinition_missingFlowContents_tsDefinitionUpdatedAndExcepti
     }
 
     @Test
-    public void customTsDefinition_oldFlowContents_tsDefinitionUpdatedAndExceptionThrown()
+    public void customTsDefinition_oldFlowContents_tsDefinitionUpdatedAndWarningLogged()
             throws Exception {
         Path typesTSfile = new File(outputFolder, "types.d.ts").toPath();
         String originalContent = "import type { SchemaObject } from \"../../types\";"
                 + System.lineSeparator() + readPreviousContent();
         Files.writeString(typesTSfile, originalContent);
-        ExecutionFailedException exception = Assert.assertThrows(
-                ExecutionFailedException.class,
-                taskGenerateTsDefinitions::execute);
-        MatcherAssert.assertThat(exception.getMessage(), CoreMatchers
-                .containsString(TaskGenerateTsDefinitions.UPDATE_MESSAGE));
+
+        MockLogger logger = new MockLogger();
+        try (MockedStatic<AbstractTaskClientGenerator> client = Mockito
+                .mockStatic(AbstractTaskClientGenerator.class,
+                        Mockito.CALLS_REAL_METHODS)) {
+            client.when(() -> AbstractTaskClientGenerator.log())
+                    .thenReturn(logger);
+            taskGenerateTsDefinitions.execute();
+        }
+        Assert.assertTrue(logger.getLogs()
+                .contains(TaskGenerateTsDefinitions.UPDATE_MESSAGE));
+
         Assert.assertFalse(
                 "Should not generate types.d.ts when already existing",
                 taskGenerateTsDefinitions.shouldGenerate());
@@ -248,6 +265,35 @@ public void customTsDefinition_oldFlowContents_tsDefinitionUpdatedAndExceptionTh
         assertBackupFileCreated(originalContent);
     }
 
+    @Test
+    public void contentUpdateForSecondTime_tsDefinitionUpdatedAndWarningLoggedOnce()
+            throws Exception {
+        Path typesTSfile = new File(outputFolder, "types.d.ts").toPath();
+        String originalContent = "import type { SchemaObject } from \"../../types\";"
+                + System.lineSeparator() + readPreviousContent();
+        Files.writeString(typesTSfile, originalContent);
+
+        MockLogger logger = new MockLogger();
+        try (MockedStatic<AbstractTaskClientGenerator> client = Mockito
+                .mockStatic(AbstractTaskClientGenerator.class,
+                        Mockito.CALLS_REAL_METHODS)) {
+            client.when(() -> AbstractTaskClientGenerator.log())
+                    .thenReturn(logger);
+            taskGenerateTsDefinitions.execute();
+
+            Assert.assertTrue(logger.getLogs()
+                    .contains(TaskGenerateTsDefinitions.UPDATE_MESSAGE));
+
+            logger.clearLogs();
+
+            Files.writeString(typesTSfile, originalContent);
+
+            taskGenerateTsDefinitions.execute();
+        }
+        Assert.assertFalse(logger.getLogs()
+                .contains(TaskGenerateTsDefinitions.UPDATE_MESSAGE));
+    }
+
     @Test
     public void customTsDefinition_flowContents_tsDefinitionNotUpdated()
             throws Exception {
@@ -366,9 +412,12 @@ public void customTsDefinition_differentCSSModuleDefinition_tsDefinitionNotUpdat
 
     private void assertBackupFileCreated(String originalContent)
             throws IOException {
-        File backupFile = new File(
-                taskGenerateTsDefinitions.getGeneratedFile().getParent(),
-                TS_DEFINITIONS + ".flowBackup");
+        File[] backups = taskGenerateTsDefinitions.getGeneratedFile()
+                .getParentFile()
+                .listFiles(file -> file.getName().endsWith(".bak"));
+        Assert.assertEquals(1, backups.length);
+
+        File backupFile = backups[0];
         Assert.assertTrue("Original types.d.ts backup should exist",
                 backupFile.exists());
         Assert.assertEquals(originalContent,