Repository: incubator-beam Updated Branches: refs/heads/master 75f54682a -> 71c69b31b
[BEAM-764] Remove cloneAs from PipelineOptions Project: http://git-wip-us.apache.org/repos/asf/incubator-beam/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-beam/commit/71c69b31 Tree: http://git-wip-us.apache.org/repos/asf/incubator-beam/tree/71c69b31 Diff: http://git-wip-us.apache.org/repos/asf/incubator-beam/diff/71c69b31 Branch: refs/heads/master Commit: 71c69b31b6894064bf8111007f947150ff725528 Parents: 75f5468 Author: Pei He <pe...@google.com> Authored: Mon Oct 17 14:13:42 2016 -0700 Committer: Luke Cwik <lc...@google.com> Committed: Tue Oct 18 10:56:43 2016 -0700 ---------------------------------------------------------------------- .../beam/sdk/options/PipelineOptions.java | 8 ---- .../sdk/options/PipelineOptionsFactory.java | 1 - .../sdk/options/ProxyInvocationHandler.java | 22 ---------- .../beam/sdk/options/PipelineOptionsTest.java | 43 -------------------- 4 files changed, 74 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/71c69b31/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptions.java ---------------------------------------------------------------------- diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptions.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptions.java index 3d6cad6..5588543 100644 --- a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptions.java +++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptions.java @@ -211,14 +211,6 @@ public interface PipelineOptions extends HasDisplayData { <T extends PipelineOptions> T as(Class<T> kls); /** - * Makes a deep clone of this object, and transforms the cloned object into the specified - * type {@code kls}. See {@link #as} for more information about the conversion. - * - * <p>Properties that are marked with {@code @JsonIgnore} will not be cloned. - */ - <T extends PipelineOptions> T cloneAs(Class<T> kls); - - /** * The pipeline runner that will be used to execute the pipeline. * For registered runners, the class name can be specified, otherwise the fully * qualified name needs to be specified. http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/71c69b31/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java ---------------------------------------------------------------------- diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java index 1c8a835..7206b11 100644 --- a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java +++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java @@ -989,7 +989,6 @@ public class PipelineOptionsFactory { // Ignore methods on the base PipelineOptions interface. try { methods.add(iface.getMethod("as", Class.class)); - methods.add(iface.getMethod("cloneAs", Class.class)); methods.add(iface.getMethod("populateDisplayData", DisplayData.Builder.class)); } catch (NoSuchMethodException | SecurityException e) { throw new RuntimeException(e); http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/71c69b31/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ProxyInvocationHandler.java ---------------------------------------------------------------------- diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ProxyInvocationHandler.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ProxyInvocationHandler.java index 47d7cee..a77dcc6 100644 --- a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ProxyInvocationHandler.java +++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ProxyInvocationHandler.java @@ -134,10 +134,6 @@ class ProxyInvocationHandler implements InvocationHandler, HasDisplayData { @SuppressWarnings("unchecked") Class<? extends PipelineOptions> clazz = (Class<? extends PipelineOptions>) args[0]; return as(clazz); - } else if (args != null && "cloneAs".equals(method.getName()) && args[0] instanceof Class) { - @SuppressWarnings("unchecked") - Class<? extends PipelineOptions> clazz = (Class<? extends PipelineOptions>) args[0]; - return cloneAs(proxy, clazz); } else if (args != null && "populateDisplayData".equals(method.getName()) && args[0] instanceof DisplayData.Builder) { @SuppressWarnings("unchecked") @@ -223,24 +219,6 @@ class ProxyInvocationHandler implements InvocationHandler, HasDisplayData { } /** - * Backing implementation for {@link PipelineOptions#cloneAs(Class)}. - * - * @return A copy of the PipelineOptions. - */ - synchronized <T extends PipelineOptions> T cloneAs(Object proxy, Class<T> iface) { - PipelineOptions clonedOptions; - try { - clonedOptions = MAPPER.readValue(MAPPER.writeValueAsBytes(proxy), PipelineOptions.class); - } catch (IOException e) { - throw new IllegalStateException("Failed to serialize the pipeline options to JSON.", e); - } - for (Class<? extends PipelineOptions> knownIface : knownInterfaces) { - clonedOptions.as(knownIface); - } - return clonedOptions.as(iface); - } - - /** * Returns true if the other object is a ProxyInvocationHandler or is a Proxy object and has the * same ProxyInvocationHandler as this. * http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/71c69b31/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsTest.java ---------------------------------------------------------------------- diff --git a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsTest.java b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsTest.java index 012a5b0..7a3bd99 100644 --- a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsTest.java +++ b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsTest.java @@ -17,17 +17,9 @@ */ package org.apache.beam.sdk.options; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNotSame; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; import com.fasterxml.jackson.annotation.JsonIgnore; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.collect.Lists; -import com.google.common.collect.Sets; -import java.io.IOException; import java.util.List; import java.util.Set; import org.junit.Rule; @@ -78,39 +70,4 @@ public class PipelineOptionsTest { BaseTestOptions options = PipelineOptionsFactory.create().as(BaseTestOptions.class); assertNotNull(options); } - - @Test - public void testCloneAs() throws IOException { - DerivedTestOptions options = PipelineOptionsFactory.create().as(DerivedTestOptions.class); - options.setBaseValue(Lists.<Boolean>newArrayList()); - options.setIgnoredValue(Sets.<String>newHashSet()); - options.getIgnoredValue().add("ignoredString"); - options.setDerivedValue(0); - - BaseTestOptions clonedOptions = options.cloneAs(BaseTestOptions.class); - assertNotSame(clonedOptions, options); - assertNotSame(clonedOptions.getBaseValue(), options.getBaseValue()); - - clonedOptions.getBaseValue().add(true); - assertFalse(clonedOptions.getBaseValue().isEmpty()); - assertTrue(options.getBaseValue().isEmpty()); - - assertNull(clonedOptions.getIgnoredValue()); - - ObjectMapper mapper = new ObjectMapper(); - mapper.readValue(mapper.writeValueAsBytes(clonedOptions), PipelineOptions.class); - } - - @Test - public void testCloneAsConflicted() throws Exception { - DerivedTestOptions options = PipelineOptionsFactory.create().as(DerivedTestOptions.class); - options.setBaseValue(Lists.<Boolean>newArrayList()); - options.setIgnoredValue(Sets.<String>newHashSet()); - options.getIgnoredValue().add("ignoredString"); - options.setDerivedValue(0); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("incompatible return types"); - options.cloneAs(ConflictedTestOptions.class); - } }