Skip to content

Commit

Permalink
Provide a deep config copy from ConfigHelper.use()
Browse files Browse the repository at this point in the history
The config map prepared by ConfigHelper is a mix from several configuration levels. The lowest config level
(DefaultValueCache) is shared between several ConfigHelper invocations. In case a Map or Collection which is
inherited from the DefaultValueCache level gets modified, this is also visible for all subsequent steps. This
causes trouble and situation which are hard to debug.

With this change here each invocation of ConfigHelper.use() provides a deep defensive copy. With that we can
ensure that there is no configuration update from one step to another.
  • Loading branch information
marcusholl committed May 14, 2019
1 parent f9047bc commit 7a7fd3e
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/com/sap/piper/ConfigurationHelper.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,11 @@ class ConfigurationHelper implements Serializable {
handleValidationFailures()
MapUtils.traverse(config, { v -> (v instanceof GString) ? v.toString() : v })
if(config.verbose) step.echo "[${name}] Configuration: ${config}"
return config
return MapUtils.deepCopy(config)
}



/* private */ def getConfigPropertyNested(key) {
return getConfigPropertyNested(config, key)
}
Expand Down
34 changes: 34 additions & 0 deletions src/com/sap/piper/MapUtils.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,38 @@ class MapUtils implements Serializable {
}
m.putAll(updates)
}

static deepCopy(Map original) {
Map copy = [:]
for (def e : original.entrySet()) {
if(e.value == null) {
copy.put(e.key, e.value)
} else {
copy.put(e.key, deepCopy(e.value))
}
}
copy
}

static deepCopy(Set original) {
Set copy = []
for(def e : original)
copy << deepCopy(e)
copy
}

static deepCopy(List original) {
List copy = []
for(def e : original)
copy << deepCopy(e)
copy
}

/*
* In fact not a copy, but a catch all for everything not matching
* with the other signatures
*/
static deepCopy(def original) {
original
}
}
31 changes: 31 additions & 0 deletions test/groovy/com/sap/piper/MapUtilsTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,35 @@ class MapUtilsTest {
MapUtils.traverse(m, { s -> (s.startsWith('x')) ? "replaced" : s})
assert m == [a: 'replaced', m: [b: 'replaced', c: 'otherString']]
}

@Test
void testDeepCopy() {

List l = ['a', 'b', 'c']

def original = [
list: l,
set: (Set)['1', '2'],
nextLevel: [
list: ['x', 'y'],
duplicate: l,
set: (Set)[9, 8, 7]
]
]

def copy = MapUtils.deepCopy(original)

assert ! copy.is(original)
assert ! copy.list.is(original.list)
assert ! copy.set.is(original.set)
assert ! copy.nextLevel.list.is(original.nextLevel.list)
assert ! copy.nextLevel.set.is(original.nextLevel.set)
assert ! copy.nextLevel.duplicate.is(original.nextLevel.duplicate)

// Within the original identical list is used twice, but the
// assuption is that there are different lists in the copy.
assert ! copy.nextLevel.duplicate.is(copy.list)

assert copy == original
}
}

0 comments on commit 7a7fd3e

Please sign in to comment.