Support lists in commonPipelineEnvironment (#1797)

* Enable write Lists

* Enable read

* Add tests

* Fix unit test

* CodeClimate Refactor

* Add comment

pkg/config/config_test.go

@@ -154,6 +154,7 @@ steps:
 				Name:        "pe1",
 				Scope:       []string{"STEPS"},
 				ResourceRef: []ResourceReference{{Name: "commonPipelineEnvironment", Param: "test_pe1"}},
+				Type:        "string",
 			},
 		}
 		secretMetadata := []StepSecrets{

pkg/config/stepmeta.go

@@ -2,11 +2,13 @@ package config
 
 import (
 	"bytes"
+	"encoding/json"
 	"fmt"
 	"io"
 	"io/ioutil"
 	"path/filepath"
 
+	"github.com/SAP/jenkins-library/pkg/log"
 	"github.com/SAP/jenkins-library/pkg/piperenv"
 
 	"github.com/ghodss/yaml"
@@ -359,9 +361,7 @@ func (m *StepData) GetResourceParameters(path, name string) map[string]interface
 	for _, param := range m.Spec.Inputs.Parameters {
 		for _, res := range param.ResourceRef {
 			if res.Name == name {
-				if val := piperenv.GetParameter(filepath.Join(path, name), res.Param); len(val) > 0 {
-					resourceParams[param.Name] = val
-				}
+				resourceParams = getParameterValue(path, name, res, param)
 			}
 		}
 	}
@@ -369,6 +369,23 @@ func (m *StepData) GetResourceParameters(path, name string) map[string]interface
 	return resourceParams
 }
 
+func getParameterValue(path, name string, res ResourceReference, param StepParameters) map[string]interface{} {
+	resourceParams := map[string]interface{}{}
+	if val := piperenv.GetParameter(filepath.Join(path, name), res.Param); len(val) > 0 {
+		if param.Type != "string" {
+			var unmarshalledValue interface{}
+			err := json.Unmarshal([]byte(val), &unmarshalledValue)
+			if err != nil {
+				log.Entry().Debugf("Failed to unmarshal: %v", val)
+			}
+			resourceParams[param.Name] = unmarshalledValue
+		} else {
+			resourceParams[param.Name] = val
+		}
+	}
+	return resourceParams
+}
+
 func envVarsAsMap(envVars []EnvVar) map[string]string {
 	e := map[string]string{}
 	for _, v := range envVars {

pkg/config/stepmeta_test.go

@@ -527,10 +527,17 @@ func TestGetResourceParameters(t *testing.T) {
 			in: StepData{
 				Spec: StepSpec{Inputs: StepInputs{Parameters: []StepParameters{
 					{Name: "param1", ResourceRef: []ResourceReference{{Name: "notAvailable", Param: "envparam1"}}},
-					{Name: "param2", ResourceRef: []ResourceReference{{Name: "commonPipelineEnvironment", Param: "envparam2"}}},
+					{Name: "param2", ResourceRef: []ResourceReference{{Name: "commonPipelineEnvironment", Param: "envparam2"}}, Type: "string"},
 				}}}},
 			expected: map[string]interface{}{"param2": "val2"},
 		},
+		{
+			in: StepData{
+				Spec: StepSpec{Inputs: StepInputs{Parameters: []StepParameters{
+					{Name: "param3", ResourceRef: []ResourceReference{{Name: "commonPipelineEnvironment", Param: "envparam3"}}, Type: "[]string"},
+				}}}},
+			expected: map[string]interface{}{"param3": []interface{}{"value1", "value2"}},
+		},
 	}
 
 	dir, err := ioutil.TempDir("", "")
@@ -548,6 +555,7 @@ func TestGetResourceParameters(t *testing.T) {
 
 	ioutil.WriteFile(filepath.Join(cpeDir, "envparam1"), []byte("val1"), 0700)
 	ioutil.WriteFile(filepath.Join(cpeDir, "envparam2"), []byte("val2"), 0700)
+	ioutil.WriteFile(filepath.Join(cpeDir, "envparam3"), []byte("[\"value1\",\"value2\"]"), 0700)
 
 	for run, test := range tt {
 		t.Run(fmt.Sprintf("Run %v", run), func(t *testing.T) {

test/groovy/CommonPipelineEnvironmentTest.groovy

@@ -64,6 +64,7 @@ class CommonPipelineEnvironmentTest extends BasePiperTest {
         nullScript.commonPipelineEnvironment.originalArtifactVersion = '2.0.0'
         nullScript.commonPipelineEnvironment.setContainerProperty('image', 'myImage')
         nullScript.commonPipelineEnvironment.setValue('custom1', 'customVal1')
+        nullScript.commonPipelineEnvironment.setAbapRepositoryNames(['value1', 'value2'])
         nullScript.commonPipelineEnvironment.writeToDisk(nullScript)
 
 
@@ -71,6 +72,7 @@ class CommonPipelineEnvironmentTest extends BasePiperTest {
         assertThat(writeFileRule.files['.pipeline/commonPipelineEnvironment/originalArtifactVersion'], is('2.0.0'))
         assertThat(writeFileRule.files['.pipeline/commonPipelineEnvironment/container/image'], is('myImage'))
         assertThat(writeFileRule.files['.pipeline/commonPipelineEnvironment/custom/custom1'], is('customVal1'))
+        assertThat(writeFileRule.files['.pipeline/commonPipelineEnvironment/abap/repositoryNames'], is('[\"value1\",\"value2\"]'))
     }
 
     @Test
@@ -80,6 +82,7 @@ class CommonPipelineEnvironmentTest extends BasePiperTest {
             '.pipeline/commonPipelineEnvironment/artifactVersion',
             '.pipeline/commonPipelineEnvironment/originalArtifactVersion',
             '.pipeline/commonPipelineEnvironment/custom/custom1',
+            '.pipeline/commonPipelineEnvironment/abap/repositoryNames',
         ])
 
         nullScript.metaClass.findFiles {
@@ -95,6 +98,7 @@ class CommonPipelineEnvironmentTest extends BasePiperTest {
             '.pipeline/commonPipelineEnvironment/artifactVersion': '1.0.0',
             '.pipeline/commonPipelineEnvironment/originalArtifactVersion': '2.0.0',
             '.pipeline/commonPipelineEnvironment/custom': 'customVal1',
+            '.pipeline/commonPipelineEnvironment/abap/repositoryNames': '[\"value1\",\"value2\"]',
         ])
 
         nullScript.commonPipelineEnvironment.readFromDisk(nullScript)
@@ -102,6 +106,7 @@ class CommonPipelineEnvironmentTest extends BasePiperTest {
         assertThat(nullScript.commonPipelineEnvironment.artifactVersion, is('1.0.0'))
         assertThat(nullScript.commonPipelineEnvironment.originalArtifactVersion, is('2.0.0'))
         assertThat(nullScript.commonPipelineEnvironment.valueMap['custom1'], is('customVal1'))
+        assertThat(nullScript.commonPipelineEnvironment.abapRepositoryNames, is("[\"value1\",\"value2\"]"))
     }
 
 }

vars/commonPipelineEnvironment.groovy

@@ -42,6 +42,8 @@ class commonPipelineEnvironment implements Serializable {
 
     String mtarFilePath = ""
 
+    def abapRepositoryNames = []
+
     private Map valueMap = [:]
 
     void setValue(String property, value) {
@@ -55,6 +57,9 @@ class commonPipelineEnvironment implements Serializable {
     String changeDocumentId
 
     def reset() {
+
+        abapRepositoryNames = []
+
         appContainerProperties = [:]
         artifactVersion = null
         originalArtifactVersion = null
@@ -177,13 +182,18 @@ class commonPipelineEnvironment implements Serializable {
         [filename: '.pipeline/commonPipelineEnvironment/git/commitId', property: 'gitCommitId'],
         [filename: '.pipeline/commonPipelineEnvironment/git/commitMessage', property: 'gitCommitMessage'],
         [filename: '.pipeline/commonPipelineEnvironment/mtarFilePath', property: 'mtarFilePath'],
+        [filename: '.pipeline/commonPipelineEnvironment/abap/repositoryNames', property: 'abapRepositoryNames'],
     ]
 
     void writeToDisk(script) {
 
         files.each({f  ->
             if (this[f.property] && !script.fileExists(f.filename)) {
-                script.writeFile file: f.filename, text: this[f.property]
+                if(this[f.property] instanceof String) {
+                    script.writeFile file: f.filename, text: this[f.property]
+                } else {
+                    script.writeFile file: f.filename, text: groovy.json.JsonOutput.toJson(this[f.property])
+                }
             }
         })
 
@@ -212,6 +222,8 @@ class commonPipelineEnvironment implements Serializable {
 
     void readFromDisk(script) {
 
+        // While a groovy list will be parsed into a JSON list in "writeToDisk", the function "readFromDisk" won't parse the received String
+
         files.each({f  ->
             if (script.fileExists(f.filename)) {
                 this[f.property] = script.readFile(f.filename)