From dc2e4530e2f02c9a8ae26b5e918cd4ebe7e9116a Mon Sep 17 00:00:00 2001 From: msimpson Date: Thu, 23 Apr 2015 09:26:40 -0500 Subject: [PATCH] Reusing Chainr instance produces incorrect transform fixes #122 - Elements in the WalkedPath should be temporary / stack level, the bug was that that match method on LiteralPathElement was not making a copy of itself, and was instead reusing the same LiteralPathElement from the spec across multiple calls / threads. --- .../pathelement/LiteralPathElement.java | 5 +- .../java/com/bazaarvoice/jolt/ChainrTest.java | 53 ++++++++++++++++++- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/jolt-core/src/main/java/com/bazaarvoice/jolt/common/pathelement/LiteralPathElement.java b/jolt-core/src/main/java/com/bazaarvoice/jolt/common/pathelement/LiteralPathElement.java index 2ddd8bf0..f396fe24 100644 --- a/jolt-core/src/main/java/com/bazaarvoice/jolt/common/pathelement/LiteralPathElement.java +++ b/jolt-core/src/main/java/com/bazaarvoice/jolt/common/pathelement/LiteralPathElement.java @@ -57,7 +57,10 @@ public String evaluate( WalkedPath walkedPath ) { @Override public LiteralPathElement match( String dataKey, WalkedPath walkedPath ) { - return getRawKey().equals( dataKey ) ? this : null ; + if ( getRawKey().equals( dataKey ) ) { + return new LiteralPathElement( getRawKey(), subKeys ); + } + return null; } @Override diff --git a/jolt-core/src/test/java/com/bazaarvoice/jolt/ChainrTest.java b/jolt-core/src/test/java/com/bazaarvoice/jolt/ChainrTest.java index 57d0c6a1..bab2b3a7 100644 --- a/jolt-core/src/test/java/com/bazaarvoice/jolt/ChainrTest.java +++ b/jolt-core/src/test/java/com/bazaarvoice/jolt/ChainrTest.java @@ -21,6 +21,8 @@ import com.bazaarvoice.jolt.exception.SpecException; import com.bazaarvoice.jolt.exception.TransformException; import com.bazaarvoice.jolt.chainr.transforms.ExplodingTestTransform; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -65,7 +67,7 @@ private Map newCustomJavaActivity( Class cls, Object spec ) { private List> newCustomJavaChainrSpec( Class cls, Object delegateSpec ) { List> retvalue = this.newChainrSpec(); - retvalue.add( newCustomJavaActivity( cls, delegateSpec )); + retvalue.add( newCustomJavaActivity( cls, delegateSpec ) ); return retvalue; } @@ -241,4 +243,53 @@ public void runTestCases(String testCaseName, boolean sorted ) throws IOExceptio Assert.assertNull( orderErrorMessage, orderErrorMessage ); } } + + + + + @Test + public void testReuseChainr() { + // Spec which moves "attributeMap"'s keys to a root "attributes" list. + Map specShift = JsonUtils.javason( + "{" + + "'operation':'shift'," + + "'spec' : { 'attributeMap' : { '*' : { '$' : 'attributes[#2]' } } }" + + "}" + ); + + List> chainrSpec = ImmutableList.of( specShift ); + + // Create a single Chainr from the spec + Chainr chainr = Chainr.fromSpec(chainrSpec); + + // Test input with three attributes + Map content = JsonUtils.javason( + "{ 'attributeMap' : { " + + "'attribute1' : 1, 'attribute2' : 2, 'attribute3' : 3 }" + + "}" + ); + + Object transformed = chainr.transform(content); + + // First time everything checks out + Assert.assertTrue( transformed instanceof Map ); + Map transformedMap = (Map) transformed; + Assert.assertEquals( transformedMap.get( "attributes" ), ImmutableList.of( "attribute1", "attribute2", "attribute3" ) ); + + // Create a new identical input + content = JsonUtils.javason( + "{ 'attributeMap' : { " + + "'attribute1' : 1, 'attribute2' : 2, 'attribute3' : 3 }" + + "}" + ); + + // Create a new transform from the same Chainr + transformed = chainr.transform(content); + + Assert.assertTrue( transformed instanceof Map ); + transformedMap = (Map) transformed; + // The following assert fails because attributes will have three leading null values: + // transformedMap["attributes"] == [null, null, null, "attribute1", "attribute2", "attribute3"] + Assert.assertEquals( transformedMap.get( "attributes" ), ImmutableList.of( "attribute1", "attribute2", "attribute3" ) ); + } }