From 1af3e078da65672ad9c9e32daa4168a409693776 Mon Sep 17 00:00:00 2001 From: sammcveety Date: Tue, 27 Dec 2016 06:56:53 -0800 Subject: [PATCH 1/4] Update ValueProvider.java --- .../java/org/apache/beam/sdk/options/ValueProvider.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ValueProvider.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ValueProvider.java index 93fcaf898048..a08c592b731c 100644 --- a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ValueProvider.java +++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ValueProvider.java @@ -108,6 +108,7 @@ class NestedValueProvider implements ValueProvider, Serializable { private final ValueProvider value; private final SerializableFunction translator; + private transient X cachedValue; NestedValueProvider(ValueProvider value, SerializableFunction translator) { this.value = checkNotNull(value); @@ -125,7 +126,10 @@ public static NestedValueProvider of( @Override public T get() { - return translator.apply(value.get()); + if (cachedValue == null) { + cachedValue = translator.apply(value.get()); + } + return cachedValue; } @Override From 811397877d6053d48b8e5442f6a9cac87873b3b4 Mon Sep 17 00:00:00 2001 From: Sam McVeety Date: Tue, 27 Dec 2016 07:10:07 -0800 Subject: [PATCH 2/4] Fixup --- .../main/java/org/apache/beam/sdk/options/ValueProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ValueProvider.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ValueProvider.java index a08c592b731c..b6b209c153f8 100644 --- a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ValueProvider.java +++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ValueProvider.java @@ -108,7 +108,7 @@ class NestedValueProvider implements ValueProvider, Serializable { private final ValueProvider value; private final SerializableFunction translator; - private transient X cachedValue; + private transient T cachedValue; NestedValueProvider(ValueProvider value, SerializableFunction translator) { this.value = checkNotNull(value); From 0ea9aa491a3366d1799c91ef1945bee82ee904e8 Mon Sep 17 00:00:00 2001 From: Sam McVeety Date: Tue, 27 Dec 2016 12:22:48 -0800 Subject: [PATCH 3/4] Fixup --- .../main/java/org/apache/beam/sdk/options/ValueProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ValueProvider.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ValueProvider.java index b6b209c153f8..030eed595939 100644 --- a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ValueProvider.java +++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ValueProvider.java @@ -108,7 +108,7 @@ class NestedValueProvider implements ValueProvider, Serializable { private final ValueProvider value; private final SerializableFunction translator; - private transient T cachedValue; + private transient volatile T cachedValue; NestedValueProvider(ValueProvider value, SerializableFunction translator) { this.value = checkNotNull(value); From 3fd4e0ecd5d541f4708d289992251081322bc74f Mon Sep 17 00:00:00 2001 From: Sam McVeety Date: Wed, 28 Dec 2016 13:20:39 -0800 Subject: [PATCH 4/4] Fixup --- .../beam/sdk/options/ValueProviderTest.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/ValueProviderTest.java b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/ValueProviderTest.java index ea5cc54e7dcf..f5a4354e62cf 100644 --- a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/ValueProviderTest.java +++ b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/ValueProviderTest.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import com.fasterxml.jackson.databind.ObjectMapper; @@ -289,4 +290,34 @@ public void testNestedValueProviderSerialize() throws Exception { StaticValueProvider.of("foo"), new NonSerializableTranslator()); SerializableUtils.ensureSerializable(nvp); } + + private static class SelfIncrement { + private static int counter = 0; + + public int getValue() { + return counter++; + } + } + + private static class SelfIncrementTranslator + implements SerializableFunction { + @Override + public Integer apply(SelfIncrement from) { + return from.getValue(); + } + } + + @Test + public void testNestedValueProviderCached() throws Exception { + SelfIncrement increment = new SelfIncrement(); + ValueProvider nvp = NestedValueProvider.of( + StaticValueProvider.of(increment), new SelfIncrementTranslator()); + Integer originalValue = nvp.get(); + Integer cachedValue = nvp.get(); + Integer incrementValue = increment.getValue(); + Integer secondCachedValue = nvp.get(); + assertEquals(originalValue, cachedValue); + assertEquals(secondCachedValue, cachedValue); + assertNotEquals(originalValue, incrementValue); + } }