#471 - Fixing Help/Version Command Not Working Properly (#472)

mageddo · web-flow · commit c99366c1a035 · 2024-06-11T10:45:22.000-03:00
* testing app exit

* testing and fixing bug

* fixing and testing mapper

* testing and fixing bug

* refactoring

* one more test

* fixing test

* release notes

* [Gradle Release Plugin] - new version commit:  '3.19.3-snapshot'.

* refactoring and testing

* fixing tests

* fixing test
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
@@ -1,3 +1,6 @@
+## 3.19.3
+* #471: Fixing Help/Version Command Not Working Properly
+
 ## 3.19.2
 * Config module domain refactoring, see #470
 
diff --git a/gradle.properties b/gradle.properties
@@ -1 +1 @@
-version=3.19.2-snapshot
+version=3.19.3-snapshot
diff --git a/src/main/java/com/mageddo/dnsproxyserver/App.java b/src/main/java/com/mageddo/dnsproxyserver/App.java
@@ -1,35 +1,63 @@
 package com.mageddo.dnsproxyserver;
 
-import com.mageddo.dnsproxyserver.application.AppSettings;
+import com.mageddo.dnsproxyserver.application.LogSettings;
 import com.mageddo.dnsproxyserver.config.Config;
 import com.mageddo.dnsproxyserver.config.application.Configs;
 import com.mageddo.dnsproxyserver.config.dataprovider.ConfigDAOCmdArgs;
+import com.mageddo.dnsproxyserver.config.dataprovider.vo.ConfigFlag;
 import com.mageddo.dnsproxyserver.di.Context;
-import org.apache.commons.lang3.BooleanUtils;
 
 public class App {
+
+  private final String[] args;
+  private Config config;
+
+  public App(String[] args) {
+    this.args = args;
+  }
+
   public static void main(String[] args) {
+    new App(args).start();
+  }
 
-    ConfigDAOCmdArgs.setArgs(args);
+  void start() {
+
+    this.checkExitCommands();
+
+    this.config = this.findConfig(args);
 
-    final var config = Configs.getInstance();
+    this.setupLogs();
 
-    checkExitCommands(config);
+    this.startContext();
 
-    new AppSettings().setupLogs(config);
+    // todo install as service
+  }
 
+  Config findConfig(String[] args) {
+    ConfigDAOCmdArgs.setArgs(args);
+    return Configs.getInstance();
+  }
+
+  void setupLogs() {
+    new LogSettings().setupLogs(this.config);
+  }
+
+  void startContext() {
     final var context = Context.create();
 
     // start webserver
     // start dns server
     context.start();
-
-    // todo install as service
   }
 
-  static void checkExitCommands(Config config) {
-    if (BooleanUtils.isTrue(config.isHelpCmd()) || config.isVersionCmd()) {
-      System.exit(0);
+  void checkExitCommands() {
+    final var flags = ConfigFlag.parse(this.args);
+    if (flags.isHelp() || flags.isVersion()) {
+      exitGracefully();
     }
   }
+
+  void exitGracefully() {
+    System.exit(0);
+  }
 }
diff --git a/src/main/java/com/mageddo/dnsproxyserver/application/LogSettings.java b/src/main/java/com/mageddo/dnsproxyserver/application/LogSettings.java
@@ -11,7 +11,7 @@
 @Slf4j
 @RequiredArgsConstructor
 @Singleton
-public class AppSettings {
+public class LogSettings {
 
   /**
    * Injetando na mão, pois precisa ser feito antes de iniciar o contexto.
diff --git a/src/main/java/com/mageddo/dnsproxyserver/config/Config.java b/src/main/java/com/mageddo/dnsproxyserver/config/Config.java
@@ -71,10 +71,6 @@ public class Config {
 
   private Boolean dockerSolverHostMachineFallbackActive;
 
-  private boolean helpCmd;
-
-  private boolean versionCmd;
-
   private SolverRemote solverRemote;
 
   @JsonIgnore
diff --git a/src/main/java/com/mageddo/dnsproxyserver/config/dataprovider/ConfigDAOCmdArgs.java b/src/main/java/com/mageddo/dnsproxyserver/config/dataprovider/ConfigDAOCmdArgs.java
@@ -1,11 +1,8 @@
 package com.mageddo.dnsproxyserver.config.dataprovider;
 
 import com.mageddo.dnsproxyserver.config.Config;
-import com.mageddo.dnsproxyserver.config.SolverRemote;
-import com.mageddo.dnsproxyserver.config.dataprovider.mapper.ConfigFieldsValuesMapper;
+import com.mageddo.dnsproxyserver.config.dataprovider.mapper.ConfigFlagMapper;
 import com.mageddo.dnsproxyserver.config.dataprovider.vo.ConfigFlag;
-import com.mageddo.dnsproxyserver.utils.Booleans;
-import com.mageddo.utils.Files;
 import lombok.RequiredArgsConstructor;
 
 import javax.inject.Inject;
@@ -19,7 +16,7 @@ public class ConfigDAOCmdArgs implements ConfigDAO {
 
   @Override
   public Config find() {
-    return toConfig(this.findRaw());
+    return ConfigFlagMapper.toConfig(this.findRaw());
   }
 
   public ConfigFlag findRaw() {
@@ -35,31 +32,6 @@ public static void setArgs(String[] args) {
     ConfigDAOCmdArgs.args = args;
   }
 
-  static Config toConfig(ConfigFlag config) {
-    return Config.builder()
-      .configPath(Files.pathOf(config.getConfigPath()))
-      .registerContainerNames(config.getRegisterContainerNames())
-      .domain(config.getDomain())
-      .logFile(config.getLogToFile())
-      .logLevel(ConfigFieldsValuesMapper.mapLogLevelFrom(config.getLogLevel()))
-      .dockerHost(config.getDockerHost())
-      .hostMachineHostname(config.getHostMachineHostname())
-      .dpsNetworkAutoConnect(config.getDpsNetworkAutoConnect())
-      .noEntriesResponseCode(config.getNoEntriesResponseCode())
-      .dockerSolverHostMachineFallbackActive(config.getDockerSolverHostMachineFallbackActive())
-      .resolvConfOverrideNameServers(config.getResolvConfOverrideNameServers())
-      .mustConfigureDpsNetwork(config.getDpsNetwork())
-      .webServerPort(config.getWebServerPort())
-      .dnsServerPort(config.getDnsServerPort())
-      .defaultDns(config.getDefaultDns())
-      .solverRemote(SolverRemote
-        .builder()
-        .active(Booleans.reverseWhenNotNull(config.getNoRemoteServers()))
-        .build()
-      )
-      .build();
-  }
-
   static String[] getArgs() {
     return args;
   }
diff --git a/src/main/java/com/mageddo/dnsproxyserver/config/dataprovider/mapper/ConfigFlagMapper.java b/src/main/java/com/mageddo/dnsproxyserver/config/dataprovider/mapper/ConfigFlagMapper.java
@@ -0,0 +1,34 @@
+package com.mageddo.dnsproxyserver.config.dataprovider.mapper;
+
+import com.mageddo.dnsproxyserver.config.Config;
+import com.mageddo.dnsproxyserver.config.SolverRemote;
+import com.mageddo.dnsproxyserver.config.dataprovider.vo.ConfigFlag;
+import com.mageddo.dnsproxyserver.utils.Booleans;
+import com.mageddo.utils.Files;
+
+public class ConfigFlagMapper {
+  public static Config toConfig(ConfigFlag config) {
+    return Config.builder()
+      .configPath(Files.pathOf(config.getConfigPath()))
+      .registerContainerNames(config.getRegisterContainerNames())
+      .domain(config.getDomain())
+      .logFile(config.getLogToFile())
+      .logLevel(ConfigFieldsValuesMapper.mapLogLevelFrom(config.getLogLevel()))
+      .dockerHost(config.getDockerHost())
+      .hostMachineHostname(config.getHostMachineHostname())
+      .dpsNetworkAutoConnect(config.getDpsNetworkAutoConnect())
+      .noEntriesResponseCode(config.getNoEntriesResponseCode())
+      .dockerSolverHostMachineFallbackActive(config.getDockerSolverHostMachineFallbackActive())
+      .resolvConfOverrideNameServers(config.getResolvConfOverrideNameServers())
+      .mustConfigureDpsNetwork(config.getDpsNetwork())
+      .webServerPort(config.getWebServerPort())
+      .dnsServerPort(config.getDnsServerPort())
+      .defaultDns(config.getDefaultDns())
+      .solverRemote(SolverRemote
+        .builder()
+        .active(Booleans.reverseWhenNotNull(config.getNoRemoteServers()))
+        .build()
+      )
+      .build();
+  }
+}
diff --git a/src/main/java/com/mageddo/dnsproxyserver/config/dataprovider/vo/ConfigFlag.java b/src/main/java/com/mageddo/dnsproxyserver/config/dataprovider/vo/ConfigFlag.java
@@ -172,7 +172,7 @@ this way you will probably not have resolution issues by acl (implies dps-networ
     description = "This message (default false)",
     usageHelp = true
   )
-  private Boolean help;
+  private boolean help;
 
   @Option(
     names = {"-docker-solver-host-machine-fallback", "--docker-solver-host-machine-fallback"},
diff --git a/src/test/java/com/mageddo/dnsproxyserver/AppCompTest.java b/src/test/java/com/mageddo/dnsproxyserver/AppCompTest.java
@@ -0,0 +1,63 @@
+package com.mageddo.dnsproxyserver;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+
+@ExtendWith(MockitoExtension.class)
+class AppCompTest {
+
+  App app;
+
+  @Test
+  void mustExitWhenHelpCmd() {
+    // arrange
+    final var args = new String[]{"--help"};
+    this.app = spy(new App(args));
+
+    final var expectedException = this.mockExitMethod();
+
+    // act
+    final var exception = assertThrows(RuntimeException.class, () -> this.app.start());
+
+    // assert
+    assertEquals(expectedException.getMessage(), exception.getMessage());
+    verify(this.app, never()).setupLogs();
+    verify(this.app, never()).findConfig(any());
+  }
+
+
+  @Test
+  void mustExitWhenVerrsionCmd() {
+    // arrange
+    final var args = new String[]{"--version"};
+    this.app = spy(new App(args));
+
+    final var expectedException = this.mockExitMethod();
+
+    // act
+    final var exception = assertThrows(RuntimeException.class, () -> this.app.start());
+
+    // assert
+    assertEquals(expectedException.getMessage(), exception.getMessage());
+    verify(this.app, never()).setupLogs();
+  }
+
+  RuntimeException mockExitMethod() {
+    final var expectedException = new RuntimeException("must exit");
+    doThrow(expectedException)
+      .when(this.app)
+      .exitGracefully()
+    ;
+    return expectedException;
+  }
+
+}
diff --git a/src/test/java/com/mageddo/dnsproxyserver/AppSettingsTest.java b/src/test/java/com/mageddo/dnsproxyserver/AppSettingsTest.java
@@ -1,6 +1,6 @@
 package com.mageddo.dnsproxyserver;
 
-import com.mageddo.dnsproxyserver.application.AppSettings;
+import com.mageddo.dnsproxyserver.application.LogSettings;
 import testing.templates.ConfigTemplates;
 import com.mageddo.logback.LogbackUtils;
 import org.junit.jupiter.api.Test;
@@ -14,7 +14,7 @@ void mustLogLevelInSl4jConvetion(){
     final var config = ConfigTemplates.defaultWithoutId();
 
     // act
-    new AppSettings().setupLogs(config);
+    new LogSettings().setupLogs(config);
 
     // assert
     assertEquals("WARN", LogbackUtils.getLogLevel("com.mageddo").levelStr);
diff --git a/src/test/java/com/mageddo/dnsproxyserver/config/flags/ConfigFlagTest.java b/src/test/java/com/mageddo/dnsproxyserver/config/flags/ConfigFlagTest.java
@@ -47,7 +47,7 @@ void mustPrintHelp() {
         readString("/flags-test/002.txt"),
         sw.toString().replaceAll("\r\n", "\n")
     );
-    assertTrue(config.getHelp());
+    assertTrue(config.isHelp());
   }
 
   @Test
diff --git a/src/test/java/testing/templates/ConfigFlagTemplates.java b/src/test/java/testing/templates/ConfigFlagTemplates.java
@@ -12,4 +12,12 @@ public static ConfigFlag build() {
   public static ConfigFlag defaultWithConfigPath(Path path) {
     return ConfigFlag.parse(new String[]{"--conf-path", path.toString()});
   }
+
+  public static ConfigFlag withHelpFlag(){
+    return ConfigFlag.parse(new String[]{"--help"});
+  }
+
+  public static ConfigFlag withVersionFlag() {
+    return ConfigFlag.parse(new String[]{"--version"});
+  }
 }
diff --git a/src/test/java/testing/templates/ConfigTemplates.java b/src/test/java/testing/templates/ConfigTemplates.java
@@ -9,6 +9,7 @@
 import java.nio.file.Paths;
 
 public class ConfigTemplates {
+
   public static Config defaultWithoutId() {
     return defaultBuilder()
       .build();
diff --git a/src/test/resources/configs-test/001.json b/src/test/resources/configs-test/001.json
@@ -4,7 +4,6 @@
   "dockerSolverHostMachineFallbackActive" : true,
   "domain" : "docker",
   "dpsNetworkAutoConnect" : false,
-  "helpCmd" : false,
   "hostMachineHostname" : "host.docker",
   "logFile" : "console",
   "logLevel" : "INFO",
@@ -27,6 +26,5 @@
       "testDelay" : 20.0
     }
   },
-  "versionCmd" : false,
   "webServerPort" : 5380
 }
diff --git a/src/test/resources/configs-test/004.json b/src/test/resources/configs-test/004.json
@@ -4,7 +4,6 @@
   "dockerSolverHostMachineFallbackActive" : null,
   "domain" : "acme",
   "dpsNetworkAutoConnect" : true,
-  "helpCmd" : false,
   "hostMachineHostname" : "batata.com",
   "logFile" : "/var/log/dns-proxy-server.log",
   "logLevel" : "DEBUG",
@@ -23,6 +22,5 @@
       "testDelay" : 60.0
     }
   },
-  "versionCmd" : false,
   "webServerPort" : 9393
 }
diff --git a/src/test/resources/flags-test/001.json b/src/test/resources/flags-test/001.json
@@ -17,6 +17,6 @@
   "domain" : "docker",
   "dpsNetwork" : false,
   "dpsNetworkAutoConnect" : false,
-  "help" : null,
+  "help" : false,
   "resolvConfOverrideNameServers" : true
 }

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-version=3.19.2-snapshot`
	`1`	`+version=3.19.3-snapshot`
Original file line number	Diff line number	Diff line change
`@@ -172,7 +172,7 @@ this way you will probably not have resolution issues by acl (implies dps-networ`
`172`	`172`	`description = "This message (default false)",`
`173`	`173`	`usageHelp = true`
`174`	`174`	`)`
`175`		`- private Boolean help;`
	`175`	`+ private boolean help;`
`176`	`176`
`177`	`177`	`@Option(`
`178`	`178`	`names = {"-docker-solver-host-machine-fallback", "--docker-solver-host-machine-fallback"},`