fix(config/test): provide a non-null halconfigDirectory in HalconfigParserMocker (#2172)

* test(config): validate spring config behavior with HalconfigDirectoryStructure

to pave the way for some refactoring to support java 17

* refactor(config): use constructor injection in HalconfigParser

as a general improvement and also to pave the way to cleanly specify a non-null
halconfigDirectory during testing.

* refactor(config): use constructor injection in HalconfigDirectoryStructure

as a general improvement and also to pave the way to cleanly specify a non-null
halconfigDirectory during testing.

* fix(config/test): provide a non-null halconfigDirectory in HalconfigParserMocker

since tests like this fail in java 17 otherwise:

> Task :halyard-config:test

AccountServiceSpec > load an existent account node FAILED
    java.lang.NullPointerException
        at java.base/java.util.Objects.requireNonNull(Objects.java:209)
        at java.base/sun.nio.fs.UnixFileSystem.getPath(UnixFileSystem.java:263)
        at java.base/java.nio.file.Path.of(Path.java:147)
        at java.base/java.nio.file.Paths.get(Paths.java:69)
        at com.netflix.spinnaker.halyard.config.config.v1.HalconfigDirectoryStructure.getHalconfigPath(HalconfigDirectoryStructure.java:50)
        at com.netflix.spinnaker.halyard.config.config.v1.HalconfigParser.transformHalconfig(HalconfigParser.java:159)
        at com.netflix.spinnaker.halyard.config.services.v1.HalconfigParserMocker.mockHalconfigParser(HalconfigParserMocker.groovy:38)
        at com.netflix.spinnaker.halyard.config.services.v1.AccountServiceSpec.makeAccountService(AccountServiceSpec.groovy:34)
        at com.netflix.spinnaker.halyard.config.services.v1.AccountServiceSpec.load an existent account node(AccountServiceSpec.groovy:56)

* refactor(config): use RequiredArgsConstructor in HalconfigDirectoryStructure

to reduce boilerplate

Author: dbyron-sf

halyard-config/halyard-config.gradle

@@ -48,6 +48,7 @@ dependencies {
   testImplementation 'org.junit.jupiter:junit-jupiter-api'
   testImplementation 'org.spockframework:spock-core'
   testImplementation 'org.springframework:spring-test'
+  testImplementation 'org.springframework.boot:spring-boot-starter-test'
   testImplementation 'org.codehaus.groovy:groovy'
   testRuntimeOnly 'net.bytebuddy:byte-buddy'
   testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine'

halyard-config/src/main/java/com/netflix/spinnaker/halyard/config/config/v1/HalconfigDirectoryStructure.java

@@ -23,19 +23,19 @@
 import java.io.File;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import lombok.Setter;
-import org.springframework.beans.factory.annotation.Autowired;
+import lombok.RequiredArgsConstructor;
 import org.springframework.stereotype.Component;
 
 @Component
+@RequiredArgsConstructor
 public class HalconfigDirectoryStructure {
   private static ThreadLocal<String> directoryOverride = new ThreadLocal<>();
 
   public static void setDirectoryOverride(String directory) {
     directoryOverride.set(directory);
   }
 
-  @Autowired @Setter String halconfigDirectory;
+  private final String halconfigDirectory;
 
   public String getHalconfigDirectory() {
     String directory = directoryOverride.get();

halyard-config/src/main/java/com/netflix/spinnaker/halyard/config/config/v1/HalconfigParser.java

@@ -61,14 +61,24 @@
 @Slf4j
 @Component
 public class HalconfigParser {
-  @Autowired StrictObjectMapper objectMapper;
+  private final StrictObjectMapper objectMapper;
 
-  @Autowired HalconfigDirectoryStructure halconfigDirectoryStructure;
+  private final HalconfigDirectoryStructure halconfigDirectoryStructure;
 
-  @Autowired ApplicationContext applicationContext;
+  private final ApplicationContext applicationContext;
 
   private boolean useBackup = false;
 
+  @Autowired
+  public HalconfigParser(
+      StrictObjectMapper objectMapper,
+      HalconfigDirectoryStructure halconfigDirectoryStructure,
+      ApplicationContext applicationContext) {
+    this.objectMapper = objectMapper;
+    this.halconfigDirectoryStructure = halconfigDirectoryStructure;
+    this.applicationContext = applicationContext;
+  }
+
   private Yaml getYamlParser() {
     return applicationContext.getBean(Yaml.class);
   }

halyard-config/src/test/groovy/com/netflix/spinnaker/halyard/config/config/v1/HalconfigParserSpec.groovy

@@ -34,9 +34,7 @@ class HalconfigParserSpec extends Specification {
   void setup() {
     ApplicationContext applicationContext = Stub(ApplicationContext.class)
     applicationContext.getBean(Yaml.class) >> new Yaml(new SafeConstructor())
-    parser = new HalconfigParser()
-    parser.applicationContext = applicationContext
-    parser.objectMapper = new StrictObjectMapper()
+    parser = new HalconfigParser(new StrictObjectMapper(), null /* halconfigDirectoyStructure */, applicationContext)
   }
 
   void "Accept minimal config"() {

halyard-config/src/test/groovy/com/netflix/spinnaker/halyard/config/services/v1/HalconfigParserMocker.groovy

@@ -29,12 +29,18 @@ import java.nio.charset.StandardCharsets
 
 class HalconfigParserMocker extends Specification {
   HalconfigParser mockHalconfigParser(String config) {
-    def parserStub = new HalconfigParser()
     ApplicationContext applicationContext = Stub(ApplicationContext.class)
     applicationContext.getBean(Yaml.class) >> new Yaml(new SafeConstructor())
-    parserStub.objectMapper = new StrictObjectMapper()
-    parserStub.applicationContext = applicationContext
-    parserStub.halconfigDirectoryStructure = new HalconfigDirectoryStructure();
+
+    // HalconfigDirectoryStructure needs a non-null halconfigDirectory for
+    // Paths.get(getHalconfigDirectory()) to work properly.  The directory
+    // doesn't actually need to exist though.  So, instead of
+    //
+    // String halconfigDirectory = Files.createTempDirectory("halyard-test")
+    //
+    // let's use an arbitrary string
+    String halconfigDirectory = "halyard-test"
+    def parserStub = new HalconfigParser(new StrictObjectMapper(), new HalconfigDirectoryStructure(halconfigDirectory), applicationContext)
 
     def stream = new ByteArrayInputStream(config.getBytes(StandardCharsets.UTF_8))
     Halconfig halconfig = parserStub.parseHalconfig(stream)

halyard-config/src/test/java/com/netflix/spinnaker/halyard/config/config/v1/HalconfigConfigurationTest.java

@@ -0,0 +1,56 @@
+/*
+ * Copyright 2024 Salesforce, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License")
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.netflix.spinnaker.halyard.config.config.v1;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.springframework.boot.context.annotation.UserConfigurations;
+import org.springframework.boot.test.context.runner.ApplicationContextRunner;
+
+class HalconfigConfigurationTest {
+
+  private final ApplicationContextRunner runner =
+      new ApplicationContextRunner()
+          .withConfiguration(
+              UserConfigurations.of(ResourceConfig.class, HalconfigDirectoryStructure.class));
+
+  @BeforeEach
+  void init(TestInfo testInfo) {
+    System.out.println("--------------- Test " + testInfo.getDisplayName());
+  }
+
+  @Test
+  void testHalconfigDirectory() {
+    runner.run(
+        ctx -> {
+          String homeDir = System.getProperty("user.home");
+          assertThat(homeDir).isNotNull();
+
+          assertThat(ctx).hasSingleBean(HalconfigDirectoryStructure.class);
+          String halconfigDirectory = ctx.getBean("halconfigDirectory", String.class);
+          assertThat(halconfigDirectory).isNotNull();
+          assertThat(halconfigDirectory).isEqualTo(homeDir + "/.hal");
+          HalconfigDirectoryStructure halconfigDirectoyStructure =
+              ctx.getBean(HalconfigDirectoryStructure.class);
+          assertThat(halconfigDirectoyStructure.getHalconfigDirectory())
+              .isEqualTo(halconfigDirectory);
+        });
+  }
+}