Repository: incubator-beam Updated Branches: refs/heads/master 2ee444d15 -> 8462acbcb
PipelineOptions display data needs to handle array types PipelineOptions generates display data for arbitrary option types using #toString(). For array types, this gives an message like [Ljava.lang.String;@fc25934b]. Instead, we detect array types and use Arrays.deepString to build a string based on array values. Project: http://git-wip-us.apache.org/repos/asf/incubator-beam/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-beam/commit/bed5000a Tree: http://git-wip-us.apache.org/repos/asf/incubator-beam/tree/bed5000a Diff: http://git-wip-us.apache.org/repos/asf/incubator-beam/diff/bed5000a Branch: refs/heads/master Commit: bed5000a0f19312736e74725d186d16a504e5031 Parents: 2ee444d Author: Scott Wegner <sweg...@google.com> Authored: Fri Sep 30 10:05:00 2016 -0700 Committer: Luke Cwik <lc...@google.com> Committed: Mon Oct 3 09:01:36 2016 -0700 ---------------------------------------------------------------------- .../sdk/options/ProxyInvocationHandler.java | 22 +++++++++++++++++-- .../sdk/options/ProxyInvocationHandlerTest.java | 23 ++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/bed5000a/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 204ad97..aa6f500 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 @@ -292,7 +292,7 @@ class ProxyInvocationHandler implements InvocationHandler, HasDisplayData { builder.add(DisplayData.item(option.getKey(), type, value) .withNamespace(pipelineInterface)); } else { - builder.add(DisplayData.item(option.getKey(), value.toString()) + builder.add(DisplayData.item(option.getKey(), displayDataString(value)) .withNamespace(pipelineInterface)); } } @@ -321,7 +321,7 @@ class ProxyInvocationHandler implements InvocationHandler, HasDisplayData { builder.add(DisplayData.item(jsonOption.getKey(), type, value) .withNamespace(spec.getDefiningInterface())); } else { - builder.add(DisplayData.item(jsonOption.getKey(), value.toString()) + builder.add(DisplayData.item(jsonOption.getKey(), displayDataString(value)) .withNamespace(spec.getDefiningInterface())); } } @@ -330,6 +330,24 @@ class ProxyInvocationHandler implements InvocationHandler, HasDisplayData { } /** + * {@link Object#toString()} wrapper to extract display data values for various types. + */ + private String displayDataString(Object value) { + checkNotNull(value, "value cannot be null"); + if (!value.getClass().isArray()) { + return value.toString(); + } + if (!value.getClass().getComponentType().isPrimitive()) { + return Arrays.deepToString((Object[]) value); + } + + // At this point, we have some type of primitive array. Arrays.deepToString(..) requires an + // Object array, but will unwrap nested primitive arrays. + String wrapped = Arrays.deepToString(new Object[] {value}); + return wrapped.substring(1, wrapped.length() - 1); + } + + /** * Marker interface used when the original {@link PipelineOptions} interface is not known at * runtime. This can occur if {@link PipelineOptions} are deserialized from JSON. * http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/bed5000a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/ProxyInvocationHandlerTest.java ---------------------------------------------------------------------- diff --git a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/ProxyInvocationHandlerTest.java b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/ProxyInvocationHandlerTest.java index 1ba6b43..5d8ef43 100644 --- a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/ProxyInvocationHandlerTest.java +++ b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/ProxyInvocationHandlerTest.java @@ -854,6 +854,29 @@ public class ProxyInvocationHandlerTest { } @Test + public void testDisplayDataArrayValue() throws Exception { + ArrayOptions options = PipelineOptionsFactory.as(ArrayOptions.class); + options.setDeepArray(new String[][] {new String[] {"a", "b"}, new String[] {"c"}}); + options.setDeepPrimitiveArray(new int[][] {new int[] {1, 2}, new int[] {3}}); + + DisplayData data = DisplayData.from(options); + assertThat(data, hasDisplayItem("deepArray", "[[a, b], [c]]")); + assertThat(data, hasDisplayItem("deepPrimitiveArray", "[[1, 2], [3]]")); + + ArrayOptions deserializedOptions = serializeDeserialize(ArrayOptions.class, options); + DisplayData deserializedData = DisplayData.from(deserializedOptions); + assertThat(deserializedData, hasDisplayItem("deepPrimitiveArray", "[[1, 2], [3]]")); + } + + private interface ArrayOptions extends PipelineOptions { + String[][] getDeepArray(); + void setDeepArray(String[][] value); + + int[][] getDeepPrimitiveArray(); + void setDeepPrimitiveArray(int[][] value); + } + + @Test public void testDisplayDataJsonSerialization() throws IOException { FooOptions options = PipelineOptionsFactory.as(FooOptions.class); options.setFoo("bar");