From 5c76c44e7ef0c31ec4184ecb3a6601fcededce50 Mon Sep 17 00:00:00 2001 From: Scott Wegner Date: Tue, 5 Apr 2016 11:42:30 -0700 Subject: [PATCH 1/6] Move additional matchers into DisplayDataMatchers --- .../display/DisplayDataMatchers.java | 43 +++++++++++++++++ .../display/DisplayDataMatchersTest.java | 34 +++++++++++--- .../transforms/display/DisplayDataTest.java | 47 ++++++------------- 3 files changed, 85 insertions(+), 39 deletions(-) diff --git a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataMatchers.java b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataMatchers.java index d540b4b628a9..2832414256ca 100644 --- a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataMatchers.java +++ b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataMatchers.java @@ -248,10 +248,44 @@ protected String featureValueOf(DisplayData.Item actual) { }; } + /** + * Creates a matcher that matches if the examined {@link DisplayData.Item} contains the + * specified namespace. + */ + public static Matcher hasNamespace(Class namespace) { + return hasNamespace(Matchers.>is(namespace)); + } + + /** + * Creates a matcher that matches if the examined {@link DisplayData.Item} contains a namespace + * matching the specified namespace matcher. + */ + public static Matcher hasNamespace(Matcher> namespaceMatcher) { + return new FeatureMatcher>( + namespaceMatcher, "display item with namespace", "namespace") { + @Override + protected Class featureValueOf(DisplayData.Item actual) { + try { + return Class.forName(actual.getNamespace()); + } catch (ClassNotFoundException e) { + return null; + } + } + }; + } + + /** + * Creates a matcher that matches if the examined {@link DisplayData.Item} matches the + * specified type. + */ public static Matcher hasType(DisplayData.Type type) { return hasType(Matchers.is(type)); } + /** + * Creates a matcher that matches if the examined {@link DisplayData.Item} has a type + * matching the specified type matcher. + */ public static Matcher hasType(Matcher typeMatcher) { return new FeatureMatcher( typeMatcher, "with type", "type") { @@ -262,10 +296,19 @@ protected DisplayData.Type featureValueOf(DisplayData.Item actual) { }; } + /** + * Creates a matcher that matches if the examined {@link DisplayData.Item} has the specified + * value. + */ + public static Matcher hasValue(String value) { return hasValue(Matchers.is(value)); } + /** + * Creates a matcher that matches if the examined {@link DisplayData.Item} contains a value + * matching the specified value matcher. + */ public static Matcher hasValue(Matcher valueMatcher) { return new FeatureMatcher( valueMatcher, "with value", "value") { diff --git a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataMatchersTest.java b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataMatchersTest.java index 1b43ff7ea02c..f24288eb75a6 100644 --- a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataMatchersTest.java +++ b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataMatchersTest.java @@ -19,6 +19,7 @@ import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.hasDisplayItem; import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.hasKey; +import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.hasNamespace; import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.hasType; import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.hasValue; import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.includes; @@ -96,6 +97,15 @@ public void testHasValue() { assertThat(createDisplayDataWithItem("foo", "bar"), matcher); } + @Test + public void testHasNamespace() { + Matcher matcher = hasDisplayItem(hasNamespace(SampleTransform.class)); + + assertFalse(matcher.matches(DisplayData.from( + new PTransform, PCollection>(){}))); + assertThat(createDisplayDataWithItem("foo", "bar"), matcher); + } + @Test public void testIncludes() { final HasDisplayData subComponent = new HasDisplayData() { @@ -125,13 +135,23 @@ public void populateDisplayData(Builder builder) { assertThat(DisplayData.from(subComponent), matcher); } + private DisplayData createDisplayDataWithItem(final String key, final String value) { - return DisplayData.from( - new PTransform, PCollection>() { - @Override - public void populateDisplayData(Builder builder) { - builder.add(key, value); - } - }); + return DisplayData.from(new SampleTransform(key, value)); + } + + static class SampleTransform extends PTransform, PCollection> { + private final String key; + private final String value; + + SampleTransform(String key, String value) { + this.key = key; + this.value = value; + } + + @Override + public void populateDisplayData(Builder builder) { + builder.add(key, value); + } } } diff --git a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java index 4d75e267197a..4921f54f44a7 100644 --- a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java +++ b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java @@ -19,6 +19,7 @@ import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.hasDisplayItem; import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.hasKey; +import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.hasNamespace; import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.hasType; import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.hasValue; import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.includes; @@ -158,14 +159,22 @@ public void populateDisplayData(DisplayData.Builder builder) { @Test public void testItemProperties() { final Instant value = Instant.now(); - DisplayData data = DisplayData.from(new ConcreteComponent(value)); + DisplayData data = DisplayData.from(new HasDisplayData() { + @Override + public void populateDisplayData(DisplayData.Builder builder) { + builder.add("now", value) + .withLabel("the current instant") + .withLinkUrl("http://time.gov") + .withNamespace(DisplayDataTest.class); + } + }); @SuppressWarnings("unchecked") DisplayData.Item item = (DisplayData.Item) data.items().toArray()[0]; assertThat( item, allOf( - hasNamespace(Matchers.>is(ConcreteComponent.class)), + hasNamespace(DisplayDataTest.class), hasKey("now"), hasType(DisplayData.Type.TIMESTAMP), hasValue(ISO_FORMATTER.print(value)), @@ -174,19 +183,6 @@ public void testItemProperties() { hasUrl(is("http://time.gov")))); } - static class ConcreteComponent implements HasDisplayData { - private Instant value; - - ConcreteComponent(Instant value) { - this.value = value; - } - - @Override - public void populateDisplayData(DisplayData.Builder builder) { - builder.add("now", value).withLabel("the current instant").withLinkUrl("http://time.gov"); - } - } - @Test public void testUnspecifiedOptionalProperties() { DisplayData data = @@ -534,7 +530,7 @@ public void populateDisplayData(DisplayData.Builder builder) { hasItem( allOf( hasKey("alpha"), - hasNamespace(Matchers.>is(component.getClass()))))); + hasNamespace(component.getClass())))); } @Test @@ -610,28 +606,15 @@ public void testAcceptsNullOptionalValues() { @Override public void populateDisplayData(Builder builder) { builder.add("key", "value") - .withLabel(null) - .withLinkUrl(null); + .withLabel(null) + .withLinkUrl(null) + .withNamespace(null); } }); // Should not throw } - private static Matcher hasNamespace(Matcher> nsMatcher) { - return new FeatureMatcher>( - nsMatcher, "display item with namespace", "namespace") { - @Override - protected Class featureValueOf(DisplayData.Item actual) { - try { - return Class.forName(actual.getNamespace()); - } catch (ClassNotFoundException e) { - return null; - } - } - }; - } - private static Matcher hasLabel(Matcher labelMatcher) { return new FeatureMatcher( labelMatcher, "display item with label", "label") { From c7aa0e4665777334be7c210da7bee9aac6735ffb Mon Sep 17 00:00:00 2001 From: Scott Wegner Date: Tue, 5 Apr 2016 11:46:23 -0700 Subject: [PATCH 2/6] Support overriding namespace when adding display data --- .../sdk/transforms/display/DisplayData.java | 59 +++++++++++++++---- .../transforms/display/DisplayDataTest.java | 16 +++++ 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java b/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java index 63ec7fcde12c..acd6851dbd22 100644 --- a/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java +++ b/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java @@ -101,6 +101,10 @@ public String toString() { return builder.toString(); } + private static String convertNamespace(Class nsClass) { + return nsClass.getName(); + } + /** * Utility to build up display metadata from a component and its included * subcomponents. @@ -190,6 +194,15 @@ public interface ItemBuilder extends Builder { *

Specifying a null value will clear the URL if it was previously defined. */ ItemBuilder withLinkUrl(@Nullable String url); + + /** + * Adds an explicit namespace to the most-recently added display metadata. The namespace + * and key uniquely identify the display metadata. + * + *

Specifying a null value or leaving the namespace unspecified will default to + * the registering instance's class. + */ + ItemBuilder withNamespace(@Nullable Class namespace); } /** @@ -207,10 +220,11 @@ public static class Item { private final String label; private final String url; - private static Item create(String namespace, String key, Type type, T value) { + private static Item create(Class nsClass, String key, Type type, T value) { FormattedItemValue formatted = type.format(value); + String namespace = convertNamespace(nsClass); return new Item( - namespace, key, type, formatted.getLongValue(), formatted.getShortValue(), null, null); + namespace, key, type, formatted.getLongValue(), formatted.getShortValue(), null, null); } private Item( @@ -337,6 +351,12 @@ private Item withLabel(String label) { private Item withUrl(String url) { return new Item(this.ns, this.key, this.type, this.value, this.shortValue, url, this.label); } + + private Item withNamespace(Class nsClass) { + String namespace = convertNamespace(nsClass); + return new Item( + namespace, this.key, this.type, this.value, this.shortValue, this.url, this.label); + } } /** @@ -354,7 +374,7 @@ public static class Identifier { private final String key; public static Identifier of(Class namespace, String key) { - return of(namespace.getName(), key); + return of(convertNamespace(namespace), key); } public static Identifier of(String namespace, String key) { @@ -481,7 +501,6 @@ private static class InternalBuilder implements ItemBuilder { private Class latestNs; private Item latestItem; - private Identifier latestIdentifier; private InternalBuilder() { this.entries = Maps.newHashMap(); @@ -503,6 +522,8 @@ public Builder include(HasDisplayData subComponent) { @Override public Builder include(HasDisplayData subComponent, Class namespace) { checkNotNull(subComponent); + + commitLatest(); boolean newComponent = visited.add(subComponent); if (newComponent) { Class prevNs = this.latestNs; @@ -557,35 +578,47 @@ private ItemBuilder addItem(String key, Type type, T value) { checkNotNull(key); checkArgument(!key.isEmpty()); - Identifier id = Identifier.of(latestNs, key); + commitLatest(); + latestItem = Item.create(latestNs, key, type, value); + + return this; + } + + private void commitLatest() { + if (latestItem == null) { + return; + } + + Identifier id = Identifier.of(latestItem.getNamespace(), latestItem.getKey()); if (entries.containsKey(id)) { throw new IllegalArgumentException("DisplayData key already exists. All display data " + "for a component must be registered with a unique key.\nKey: " + id); } - Item item = Item.create(id.getNamespace(), key, type, value); - entries.put(id, item); - - latestItem = item; - latestIdentifier = id; - return this; + entries.put(id, latestItem); + latestItem = null; } @Override public ItemBuilder withLabel(String label) { latestItem = latestItem.withLabel(label); - entries.put(latestIdentifier, latestItem); return this; } @Override public ItemBuilder withLinkUrl(String url) { latestItem = latestItem.withUrl(url); - entries.put(latestIdentifier, latestItem); + return this; + } + + @Override + public ItemBuilder withNamespace(@Nullable Class namespace) { + latestItem = latestItem.withNamespace(namespace); return this; } private DisplayData build() { + commitLatest(); return new DisplayData(this.entries); } } diff --git a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java index 4921f54f44a7..5ab476c6f142 100644 --- a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java +++ b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java @@ -345,6 +345,22 @@ public void populateDisplayData(DisplayData.Builder builder) { }); } + @Test + public void testDuplicateKeyWithNamespaceOverrideDoesntThrow() { + DisplayData displayData = DisplayData.from( + new HasDisplayData() { + @Override + public void populateDisplayData(DisplayData.Builder builder) { + builder + .add("foo", "bar") + .add("foo", "baz") + .withNamespace(DisplayDataTest.class); + } + }); + + assertThat(displayData.items(), hasSize(2)); + } + @Test public void testToString() { HasDisplayData component = new HasDisplayData() { From 4cc601e629433e9ac7cb48a4165fa967e9a6e50a Mon Sep 17 00:00:00 2001 From: Scott Wegner Date: Tue, 5 Apr 2016 11:48:36 -0700 Subject: [PATCH 3/6] Support inferring display data type during registration --- .../sdk/transforms/display/DisplayData.java | 59 ++++++++++++++++--- .../transforms/display/DisplayDataTest.java | 46 +++++++++++++++ 2 files changed, 96 insertions(+), 9 deletions(-) diff --git a/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java b/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java index acd6851dbd22..52d1b65beb19 100644 --- a/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java +++ b/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java @@ -171,6 +171,17 @@ public interface Builder { * from the current transform or component. */ ItemBuilder add(String key, Class value); + + /** + * Register the given display metadata. The input value will be inspected to see if it conforms + * to one of the supported DisplayData types. Otherwise, it will be registered as a + * {@link DisplayData.Type#STRING}, using the {@link Object#toString()} method to retrieve the + * display value. + * + *

The added display data is identified by the specified key and namespace from the current + * transform or component. + */ + ItemBuilder add(String key, Object value); } /** @@ -199,10 +210,9 @@ public interface ItemBuilder extends Builder { * Adds an explicit namespace to the most-recently added display metadata. The namespace * and key uniquely identify the display metadata. * - *

Specifying a null value or leaving the namespace unspecified will default to - * the registering instance's class. + *

Leaving the namespace unspecified will default to the registering instance's class. */ - ItemBuilder withNamespace(@Nullable Class namespace); + ItemBuilder withNamespace(Class namespace); } /** @@ -423,13 +433,14 @@ public enum Type { STRING { @Override FormattedItemValue format(Object value) { - return new FormattedItemValue((String) value); + return new FormattedItemValue(value.toString()); } }, INTEGER { @Override FormattedItemValue format(Object value) { - return new FormattedItemValue(Long.toString((long) value)); + Number number = (Number) value; + return new FormattedItemValue(Long.toString(number.longValue())); } }, FLOAT { @@ -471,6 +482,28 @@ FormattedItemValue format(Object value) { *

Internal-only. Value objects can be safely cast to the expected Java type. */ abstract FormattedItemValue format(Object value); + + /** + * Infer the {@link Type} for the given object. + */ + static Type inferFrom(@Nullable Object value) { + if (value instanceof Integer || value instanceof Long) { + return INTEGER; + } else if (value instanceof Double || value instanceof Float) { + return FLOAT; + } else if (value instanceof Boolean) { + return BOOLEAN; + } else if (value instanceof Instant) { + return TIMESTAMP; + } else if (value instanceof Duration) { + return DURATION; + } else if (value instanceof Class) { + return JAVA_CLASS; + } else { + // default + return STRING; + } + } } static class FormattedItemValue { @@ -574,7 +607,14 @@ public ItemBuilder add(String key, Class value) { return addItem(key, Type.JAVA_CLASS, value); } - private ItemBuilder addItem(String key, Type type, T value) { + @Override + public ItemBuilder add(String key, Object value) { + checkNotNull(value); + Type type = Type.inferFrom(value); + return addItem(key, type, value); + } + + private ItemBuilder addItem(String key, Type type, Object value) { checkNotNull(key); checkArgument(!key.isEmpty()); @@ -600,19 +640,20 @@ private void commitLatest() { } @Override - public ItemBuilder withLabel(String label) { + public ItemBuilder withLabel(@Nullable String label) { latestItem = latestItem.withLabel(label); return this; } @Override - public ItemBuilder withLinkUrl(String url) { + public ItemBuilder withLinkUrl(@Nullable String url) { latestItem = latestItem.withUrl(url); return this; } @Override - public ItemBuilder withNamespace(@Nullable Class namespace) { + public ItemBuilder withNamespace(Class namespace) { + checkNotNull(namespace); latestItem = latestItem.withNamespace(namespace); return this; } diff --git a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java index 5ab476c6f142..7eab60a02eb8 100644 --- a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java +++ b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java @@ -246,6 +246,18 @@ public void populateDisplayData(DisplayData.Builder builder) { assertThat(data, includes(subComponent, namespaceOverride.getClass())); } + @Test + public void testNullNamespaceOverride() { + thrown.expect(NullPointerException.class); + + DisplayData.from(new HasDisplayData() { + @Override + public void populateDisplayData(Builder builder) { + builder.add("foo", "bar") + .withNamespace(null); + } + }); + } @Test public void testIdentifierEquality() { @@ -491,6 +503,40 @@ public void populateDisplayData(DisplayData.Builder builder) { items, hasItem(allOf(hasKey("duration"), hasType(DisplayData.Type.DURATION)))); } + @Test + public void testObjectInputType() { + DisplayData data = DisplayData.from(new HasDisplayData() { + @Override + public void populateDisplayData(Builder builder) { + builder + .add("castInteger", (Object) 1234) + .add("anonymousType", new Object() { + @Override + public String toString() { + return "foobar"; + } + }); + } + }); + + assertThat(data, hasDisplayItem("castInteger", 1234)); + assertThat(data, hasDisplayItem("anonymousType", "foobar")); + } + + @Test + public void testKnownTypeInferrence() { + assertEquals(DisplayData.Type.INTEGER, DisplayData.Type.inferFrom(1234)); + assertEquals(DisplayData.Type.INTEGER, DisplayData.Type.inferFrom(1234L)); + assertEquals(DisplayData.Type.FLOAT, DisplayData.Type.inferFrom(12.3)); + assertEquals(DisplayData.Type.FLOAT, DisplayData.Type.inferFrom(12.3f)); + assertEquals(DisplayData.Type.BOOLEAN, DisplayData.Type.inferFrom(true)); + assertEquals(DisplayData.Type.TIMESTAMP, DisplayData.Type.inferFrom(Instant.now())); + assertEquals(DisplayData.Type.DURATION, DisplayData.Type.inferFrom(Duration.millis(1234))); + assertEquals(DisplayData.Type.JAVA_CLASS, DisplayData.Type.inferFrom(DisplayDataTest.class)); + assertEquals(DisplayData.Type.STRING, DisplayData.Type.inferFrom("hello world")); + assertEquals(DisplayData.Type.STRING, DisplayData.Type.inferFrom(new Object() {})); + } + @Test public void testStringFormatting() throws IOException { final Instant now = Instant.now(); From 17667a77260e7526e9d82f2d0084c9291da36f83 Mon Sep 17 00:00:00 2001 From: Scott Wegner Date: Thu, 7 Apr 2016 10:28:27 -0700 Subject: [PATCH 4/6] Add APIs to conditionally register display data if not null/default. --- .../sdk/transforms/display/DisplayData.java | 253 +++++++++++++++--- .../transforms/display/HasDisplayData.java | 1 + .../transforms/display/DisplayDataTest.java | 152 +++++++++-- 3 files changed, 341 insertions(+), 65 deletions(-) diff --git a/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java b/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java index 52d1b65beb19..94c74db44fa9 100644 --- a/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java +++ b/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java @@ -23,6 +23,7 @@ import com.google.cloud.dataflow.sdk.transforms.DoFn; import com.google.cloud.dataflow.sdk.transforms.PTransform; import com.google.cloud.dataflow.sdk.transforms.ParDo; +import com.google.common.base.Optional; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -76,6 +77,29 @@ public static DisplayData from(HasDisplayData component) { return InternalBuilder.forRoot(component).build(); } + /** + * Infer the {@link Type} for the given object. + * + *

Use this method if the type of metadata is not known at compile time. For example: + * + * 

+   * {@code
+   * @Override
+   * public void populateDisplayData(DisplayData.Builder builder) {
+   *   Optional type = DisplayData.inferType(foo);
+   *   if (type.isPresent()) {
+   *     builder.add("foo", type.get(), foo);
+   *   }
+   * }
+   * }
+   *
+ * + * @return The inferred {@link Type}, or {@link Optional#absent()} if the type cannot be inferred, + */ + public static Optional inferType(@Nullable Object value) { + return Type.tryInferFrom(value); + } + public Collection items() { return entries.values(); } @@ -130,6 +154,20 @@ public interface Builder { */ ItemBuilder add(String key, String value); + /** + * Register the given string display data if the value is not null. + * + * @see DisplayData.Builder#add(String, String) + */ + ItemBuilder addIfNotNull(String key, @Nullable String value); + + /** + * Register the given string display data if the value is different than the specified default. + * + * @see DisplayData.Builder#add(String, String) + */ + ItemBuilder addIfNotDefault(String key, @Nullable String value, @Nullable String defaultValue); + /** * Register the given numeric display metadata. The metadata item will be registered with type * {@link DisplayData.Type#INTEGER}, and is identified by the specified key and namespace from @@ -137,6 +175,13 @@ public interface Builder { */ ItemBuilder add(String key, long value); + /** + * Register the given numeric display data if the value is different than the specified default. + * + * @see DisplayData.Builder#add(String, long) + */ + ItemBuilder addIfNotDefault(String key, long value, long defaultValue); + /** * Register the given floating point display metadata. The metadata item will be registered with * type {@link DisplayData.Type#FLOAT}, and is identified by the specified key and namespace @@ -145,12 +190,27 @@ public interface Builder { ItemBuilder add(String key, double value); /** - * Register the given floating point display metadata. The metadata item will be registered with + * Register the given floating point display data if the value is different than the specified + * default. + * + * @see DisplayData.Builder#add(String, double) + */ + ItemBuilder addIfNotDefault(String key, double value, double defaultValue); + + /** + * Register the given boolean display metadata. The metadata item will be registered with * type {@link DisplayData.Type#BOOLEAN}, and is identified by the specified key and namespace * from the current transform or component. */ ItemBuilder add(String key, boolean value); + /** + * Register the given boolean display data if the value is different than the specified default. + * + * @see DisplayData.Builder#add(String, boolean) + */ + ItemBuilder addIfNotDefault(String key, boolean value, boolean defaultValue); + /** * Register the given timestamp display metadata. The metadata item will be registered with type * {@link DisplayData.Type#TIMESTAMP}, and is identified by the specified key and namespace from @@ -158,6 +218,22 @@ public interface Builder { */ ItemBuilder add(String key, Instant value); + /** + * Register the given timestamp display data if the value is not null. + * + * @see DisplayData.Builder#add(String, Instant) + */ + ItemBuilder addIfNotNull(String key, @Nullable Instant value); + + /** + * Register the given timestamp display data if the value is different than the specified + * default. + * + * @see DisplayData.Builder#add(String, Instant) + */ + ItemBuilder addIfNotDefault( + String key, @Nullable Instant value, @Nullable Instant defaultValue); + /** * Register the given duration display metadata. The metadata item will be registered with type * {@link DisplayData.Type#DURATION}, and is identified by the specified key and namespace from @@ -165,6 +241,22 @@ public interface Builder { */ ItemBuilder add(String key, Duration value); + /** + * Register the given duration display data if the value is not null. + * + * @see DisplayData.Builder#add(String, Duration) + */ + ItemBuilder addIfNotNull(String key, @Nullable Duration value); + + /** + * Register the given duration display data if the value is different than the specified + * default. + * + * @see DisplayData.Builder#add(String, Duration) + */ + ItemBuilder addIfNotDefault( + String key, @Nullable Duration value, @Nullable Duration defaultValue); + /** * Register the given class display metadata. The metadata item will be registered with type * {@link DisplayData.Type#JAVA_CLASS}, and is identified by the specified key and namespace @@ -173,15 +265,30 @@ public interface Builder { ItemBuilder add(String key, Class value); /** - * Register the given display metadata. The input value will be inspected to see if it conforms - * to one of the supported DisplayData types. Otherwise, it will be registered as a - * {@link DisplayData.Type#STRING}, using the {@link Object#toString()} method to retrieve the - * display value. + * Register the given class display data if the value is not null. * - *

The added display data is identified by the specified key and namespace from the current - * transform or component. + * @see DisplayData.Builder#add(String, Class) */ - ItemBuilder add(String key, Object value); + ItemBuilder addIfNotNull(String key, @Nullable Class value); + + /** + * Register the given class display data if the value is different than the specified default. + * + * @see DisplayData.Builder#add(String, Class) + */ + ItemBuilder addIfNotDefault( + String key, @Nullable Class value, @Nullable Class defaultValue); + + /** + * Register the given display metadata with the specified type. + * + *

The added display data is identified by the specified key and namespace from the current + * transform or component. + * + * @throws ClassCastException if the value cannot be safely cast to the specified type. + * @see DisplayData#inferType(Object) + */ + ItemBuilder add(String key, Type type, Object value); } /** @@ -230,7 +337,7 @@ public static class Item { private final String label; private final String url; - private static Item create(Class nsClass, String key, Type type, T value) { + private static Item create(Class nsClass, String key, Type type, Object value) { FormattedItemValue formatted = type.format(value); String namespace = convertNamespace(nsClass); return new Item( @@ -483,26 +590,27 @@ FormattedItemValue format(Object value) { */ abstract FormattedItemValue format(Object value); - /** - * Infer the {@link Type} for the given object. - */ - static Type inferFrom(@Nullable Object value) { + private static Optional tryInferFrom(@Nullable Object value) { + Type type; if (value instanceof Integer || value instanceof Long) { - return INTEGER; + type = INTEGER; } else if (value instanceof Double || value instanceof Float) { - return FLOAT; + type = FLOAT; } else if (value instanceof Boolean) { - return BOOLEAN; + type = BOOLEAN; } else if (value instanceof Instant) { - return TIMESTAMP; + type = TIMESTAMP; } else if (value instanceof Duration) { - return DURATION; + type = DURATION; } else if (value instanceof Class) { - return JAVA_CLASS; + type = JAVA_CLASS; + } else if (value instanceof String) { + type = STRING; } else { - // default - return STRING; + type = null; } + + return Optional.fromNullable(type); } } @@ -533,6 +641,8 @@ private static class InternalBuilder implements ItemBuilder { private final Set visited; private Class latestNs; + + @Nullable private Item latestItem; private InternalBuilder() { @@ -555,6 +665,7 @@ public Builder include(HasDisplayData subComponent) { @Override public Builder include(HasDisplayData subComponent, Class namespace) { checkNotNull(subComponent); + checkNotNull(namespace); commitLatest(); boolean newComponent = visited.add(subComponent); @@ -571,55 +682,116 @@ public Builder include(HasDisplayData subComponent, Class namespace) { @Override public ItemBuilder add(String key, String value) { checkNotNull(value); - return addItem(key, Type.STRING, value); + return addItemIf(true, key, Type.STRING, value); + } + + @Override + public ItemBuilder addIfNotNull(String key, @Nullable String value) { + return addItemIf(value != null, key, Type.STRING, value); + } + + @Override + public ItemBuilder addIfNotDefault( + String key, @Nullable String value, @Nullable String defaultValue) { + return addItemIf(!Objects.equals(value, defaultValue), key, Type.STRING, value); } @Override public ItemBuilder add(String key, long value) { - return addItem(key, Type.INTEGER, value); + return addItemIf(true, key, Type.INTEGER, value); + } + + @Override + public ItemBuilder addIfNotDefault(String key, long value, long defaultValue) { + return addItemIf(value != defaultValue, key, Type.INTEGER, value); } @Override public ItemBuilder add(String key, double value) { - return addItem(key, Type.FLOAT, value); + return addItemIf(true, key, Type.FLOAT, value); + } + + @Override + public ItemBuilder addIfNotDefault(String key, double value, double defaultValue) { + return addItemIf(value != defaultValue, key, Type.FLOAT, value); } @Override public ItemBuilder add(String key, boolean value) { - return addItem(key, Type.BOOLEAN, value); + return addItemIf(true, key, Type.BOOLEAN, value); + } + + @Override + public ItemBuilder addIfNotDefault(String key, boolean value, boolean defaultValue) { + return addItemIf(value != defaultValue, key, Type.BOOLEAN, value); } @Override public ItemBuilder add(String key, Instant value) { checkNotNull(value); - return addItem(key, Type.TIMESTAMP, value); + return addItemIf(true, key, Type.TIMESTAMP, value); + } + + @Override + public ItemBuilder addIfNotNull(String key, @Nullable Instant value) { + return addItemIf(value != null, key, Type.TIMESTAMP, value); + } + + @Override + public ItemBuilder addIfNotDefault( + String key, @Nullable Instant value, @Nullable Instant defaultValue) { + return addItemIf(!Objects.equals(value, defaultValue), key, Type.TIMESTAMP, value); } @Override public ItemBuilder add(String key, Duration value) { checkNotNull(value); - return addItem(key, Type.DURATION, value); + return addItemIf(true, key, Type.DURATION, value); + } + + @Override + public ItemBuilder addIfNotNull(String key, @Nullable Duration value) { + return addItemIf(value != null, key, Type.DURATION, value); + } + + @Override + public ItemBuilder addIfNotDefault( + String key, @Nullable Duration value, @Nullable Duration defaultValue) { + return addItemIf(!Objects.equals(value, defaultValue), key, Type.DURATION, value); } @Override public ItemBuilder add(String key, Class value) { checkNotNull(value); - return addItem(key, Type.JAVA_CLASS, value); + return addItemIf(true, key, Type.JAVA_CLASS, value); + } + + @Override + public ItemBuilder addIfNotNull(String key, @Nullable Class value) { + return addItemIf(value != null, key, Type.JAVA_CLASS, value); + } + + @Override + public ItemBuilder addIfNotDefault( + String key, @Nullable Class value, @Nullable Class defaultValue) { + return addItemIf(!Objects.equals(value, defaultValue), key, Type.JAVA_CLASS, value); } @Override - public ItemBuilder add(String key, Object value) { + public ItemBuilder add(String key, Type type, Object value) { checkNotNull(value); - Type type = Type.inferFrom(value); - return addItem(key, type, value); + checkNotNull(type); + return addItemIf(true, key, type, value); } - private ItemBuilder addItem(String key, Type type, Object value) { + private ItemBuilder addItemIf(boolean condition, String key, Type type, Object value) { checkNotNull(key); checkArgument(!key.isEmpty()); commitLatest(); - latestItem = Item.create(latestNs, key, type, value); + if (condition) { + latestItem = Item.create(latestNs, key, type, value); + } return this; } @@ -641,20 +813,29 @@ private void commitLatest() { @Override public ItemBuilder withLabel(@Nullable String label) { - latestItem = latestItem.withLabel(label); + if (latestItem != null) { + latestItem = latestItem.withLabel(label); + } + return this; } @Override public ItemBuilder withLinkUrl(@Nullable String url) { - latestItem = latestItem.withUrl(url); + if (latestItem != null) { + latestItem = latestItem.withUrl(url); + } + return this; } @Override public ItemBuilder withNamespace(Class namespace) { checkNotNull(namespace); - latestItem = latestItem.withNamespace(namespace); + if (latestItem != null) { + latestItem = latestItem.withNamespace(namespace); + } + return this; } diff --git a/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/HasDisplayData.java b/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/HasDisplayData.java index 6ec1eca7078a..36f7a31e3d9e 100644 --- a/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/HasDisplayData.java +++ b/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/HasDisplayData.java @@ -40,6 +40,7 @@ public interface HasDisplayData { * builder * .include(subComponent) * .add("minFilter", 42) + * .addIfNotDefault("useTransactions", this.txn, false) * .add("topic", "projects/myproject/topics/mytopic") * .withLabel("Pub/Sub Topic") * .add("serviceInstance", "myservice.com/fizzbang") diff --git a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java index 7eab60a02eb8..c30632ed8ef8 100644 --- a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java +++ b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java @@ -40,6 +40,7 @@ import com.google.cloud.dataflow.sdk.transforms.display.DisplayData.Builder; import com.google.cloud.dataflow.sdk.transforms.display.DisplayData.Item; import com.google.cloud.dataflow.sdk.values.PCollection; +import com.google.common.base.Optional; import com.google.common.testing.EqualsTester; import org.hamcrest.CustomTypeSafeMatcher; @@ -59,6 +60,7 @@ import java.io.IOException; import java.util.Collection; import java.util.Map; +import java.util.Objects; import java.util.regex.Pattern; /** @@ -87,20 +89,25 @@ public void populateDisplayData(DisplayData.Builder builder) { } }; + PTransform transform = new PTransform, PCollection>() { + final Instant defaultStartTime = new Instant(0); + Instant startTime = defaultStartTime; + @Override public void populateDisplayData(DisplayData.Builder builder) { builder .include(subComponent1) .include(subComponent2) - .add("MinSproggles", 200) - .withLabel("Mimimum Required Sproggles") - .add("FireLazers", true) - .add("TimeBomb", Instant.now().plus(Duration.standardDays(1))) - .add("FilterLogic", subComponent1.getClass()) - .add("ServiceUrl", "google.com/fizzbang") - .withLinkUrl("http://www.google.com/fizzbang"); + .add("minSproggles", 200) + .withLabel("Mimimum Required Sproggles") + .add("fireLazers", true) + .addIfNotDefault("startTime", startTime, defaultStartTime) + .add("timeBomb", Instant.now().plus(Duration.standardDays(1))) + .add("filterLogic", subComponent1.getClass()) + .add("serviceUrl", "google.com/fizzbang") + .withLinkUrl("http://www.google.com/fizzbang"); } }; @@ -173,7 +180,7 @@ public void populateDisplayData(DisplayData.Builder builder) { DisplayData.Item item = (DisplayData.Item) data.items().toArray()[0]; assertThat( item, - allOf( + Matchers.allOf( hasNamespace(DisplayDataTest.class), hasKey("now"), hasType(DisplayData.Type.TIMESTAMP), @@ -199,6 +206,54 @@ public void populateDisplayData(DisplayData.Builder builder) { hasDisplayItem(allOf(hasLabel(nullValue(String.class)), hasUrl(nullValue(String.class))))); } + @Test + public void testAddIfNotDefault() { + final int defaultValue = 10; + + DisplayData data = DisplayData.from(new HasDisplayData() { + @Override + public void populateDisplayData(Builder builder) { + builder + .addIfNotDefault("isDefault", defaultValue, defaultValue) + .addIfNotDefault("notDefault", defaultValue + 1, defaultValue); + } + }); + + assertThat(data, not(hasDisplayItem(hasKey("isDefault")))); + assertThat(data, hasDisplayItem("notDefault", defaultValue + 1)); + } + + @Test + public void testAddIfNotNull() { + DisplayData data = DisplayData.from(new HasDisplayData() { + @Override + public void populateDisplayData(Builder builder) { + builder + .addIfNotNull("isNull", (Class) null) + .addIfNotNull("notNull", DisplayDataTest.class); + } + }); + + assertThat(data, not(hasDisplayItem(hasKey("isNull")))); + assertThat(data, hasDisplayItem(hasKey("notNull"))); + } + + @Test + public void testModifyingConditionalItemIsSafe() { + HasDisplayData component = new HasDisplayData() { + @Override + public void populateDisplayData(Builder builder) { + builder.addIfNotNull("nullItem", (Class) null) + .withLinkUrl("http://abc") + .withNamespace(DisplayDataTest.class) + .withLabel("Null item shoudl be safe"); + } + }; + + DisplayData.from(component); // should not throw + } + + @Test public void testIncludes() { final HasDisplayData subComponent = @@ -452,6 +507,7 @@ public int hashCode() { } @Override + @SuppressWarnings("EqualsWhichDoesntCheckParameterClass") public boolean equals(Object obj) { return true; } @@ -504,37 +560,49 @@ public void populateDisplayData(DisplayData.Builder builder) { } @Test - public void testObjectInputType() { + public void testExplicitItemType() { DisplayData data = DisplayData.from(new HasDisplayData() { @Override public void populateDisplayData(Builder builder) { builder - .add("castInteger", (Object) 1234) - .add("anonymousType", new Object() { - @Override - public String toString() { - return "foobar"; - } - }); + .add("integer", DisplayData.Type.INTEGER, 1234) + .add("string", DisplayData.Type.STRING, "foobar"); } }); - assertThat(data, hasDisplayItem("castInteger", 1234)); - assertThat(data, hasDisplayItem("anonymousType", "foobar")); + assertThat(data, hasDisplayItem("integer", 1234)); + assertThat(data, hasDisplayItem("string", "foobar")); } @Test - public void testKnownTypeInferrence() { - assertEquals(DisplayData.Type.INTEGER, DisplayData.Type.inferFrom(1234)); - assertEquals(DisplayData.Type.INTEGER, DisplayData.Type.inferFrom(1234L)); - assertEquals(DisplayData.Type.FLOAT, DisplayData.Type.inferFrom(12.3)); - assertEquals(DisplayData.Type.FLOAT, DisplayData.Type.inferFrom(12.3f)); - assertEquals(DisplayData.Type.BOOLEAN, DisplayData.Type.inferFrom(true)); - assertEquals(DisplayData.Type.TIMESTAMP, DisplayData.Type.inferFrom(Instant.now())); - assertEquals(DisplayData.Type.DURATION, DisplayData.Type.inferFrom(Duration.millis(1234))); - assertEquals(DisplayData.Type.JAVA_CLASS, DisplayData.Type.inferFrom(DisplayDataTest.class)); - assertEquals(DisplayData.Type.STRING, DisplayData.Type.inferFrom("hello world")); - assertEquals(DisplayData.Type.STRING, DisplayData.Type.inferFrom(new Object() {})); + public void testInvalidExplicitItemType() { + HasDisplayData component = new HasDisplayData() { + @Override + public void populateDisplayData(Builder builder) { + builder.add("integer", DisplayData.Type.INTEGER, "foobar"); + } + }; + + thrown.expect(ClassCastException.class); + DisplayData.from(component); + } + + @Test + public void testKnownTypeInference() { + assertThat(DisplayData.inferType(1234), hasOptionalValue(DisplayData.Type.INTEGER)); + assertThat(DisplayData.inferType(1234L), hasOptionalValue(DisplayData.Type.INTEGER)); + assertThat(DisplayData.inferType(12.3), hasOptionalValue(DisplayData.Type.FLOAT)); + assertThat(DisplayData.inferType(12.3f), hasOptionalValue(DisplayData.Type.FLOAT)); + assertThat(DisplayData.inferType(true), hasOptionalValue(DisplayData.Type.BOOLEAN)); + assertThat(DisplayData.inferType(Instant.now()), hasOptionalValue(DisplayData.Type.TIMESTAMP)); + assertThat(DisplayData.inferType(Duration.millis(1234)), + hasOptionalValue(DisplayData.Type.DURATION)); + assertThat(DisplayData.inferType(DisplayDataTest.class), + hasOptionalValue(DisplayData.Type.JAVA_CLASS)); + assertThat(DisplayData.inferType("hello world"), hasOptionalValue(DisplayData.Type.STRING)); + + assertEquals(Optional.absent(), DisplayData.inferType(null)); + assertEquals(Optional.absent(), DisplayData.inferType(new Object() {})); } @Test @@ -613,6 +681,23 @@ public void populateDisplayData(Builder builder) { }); } + @Test + public void testIncludeNullNamespace() { + final HasDisplayData subComponent = new HasDisplayData() { + @Override + public void populateDisplayData(Builder builder) { + } + }; + + thrown.expect(NullPointerException.class); + DisplayData.from(new HasDisplayData() { + @Override + public void populateDisplayData(Builder builder) { + builder.include(subComponent, null); + } + }); + } + @Test public void testNullKey() { thrown.expect(NullPointerException.class); @@ -706,4 +791,13 @@ protected String featureValueOf(DisplayData.Item actual) { } }; } + + private static Matcher> hasOptionalValue(final T value) { + return new CustomTypeSafeMatcher>("optional value") { + @Override + protected boolean matchesSafely(Optional item) { + return item.isPresent() && Objects.equals(item.get(), value); + } + }; + } } From a7a06f8f9f00e0a3611a87ebe77702830aa43da6 Mon Sep 17 00:00:00 2001 From: Scott Wegner Date: Thu, 7 Apr 2016 15:30:43 -0700 Subject: [PATCH 5/6] Remove Optional<> from API surface --- .../sdk/transforms/display/DisplayData.java | 27 +++++++------- .../transforms/display/DisplayDataTest.java | 35 ++++++------------- 2 files changed, 24 insertions(+), 38 deletions(-) diff --git a/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java b/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java index 94c74db44fa9..8dfabb1061fe 100644 --- a/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java +++ b/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java @@ -23,7 +23,6 @@ import com.google.cloud.dataflow.sdk.transforms.DoFn; import com.google.cloud.dataflow.sdk.transforms.PTransform; import com.google.cloud.dataflow.sdk.transforms.ParDo; -import com.google.common.base.Optional; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -94,9 +93,10 @@ public static DisplayData from(HasDisplayData component) { * } * * - * @return The inferred {@link Type}, or {@link Optional#absent()} if the type cannot be inferred, + * @return The inferred {@link Type}, or null if the type cannot be inferred, */ - public static Optional inferType(@Nullable Object value) { + @Nullable + public static Type inferType(@Nullable Object value) { return Type.tryInferFrom(value); } @@ -590,27 +590,26 @@ FormattedItemValue format(Object value) { */ abstract FormattedItemValue format(Object value); - private static Optional tryInferFrom(@Nullable Object value) { + @Nullable + private static Type tryInferFrom(@Nullable Object value) { Type type; if (value instanceof Integer || value instanceof Long) { - type = INTEGER; + return INTEGER; } else if (value instanceof Double || value instanceof Float) { - type = FLOAT; + return FLOAT; } else if (value instanceof Boolean) { - type = BOOLEAN; + return BOOLEAN; } else if (value instanceof Instant) { - type = TIMESTAMP; + return TIMESTAMP; } else if (value instanceof Duration) { - type = DURATION; + return DURATION; } else if (value instanceof Class) { - type = JAVA_CLASS; + return JAVA_CLASS; } else if (value instanceof String) { - type = STRING; + return STRING; } else { - type = null; + return null; } - - return Optional.fromNullable(type); } } diff --git a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java index c30632ed8ef8..9f8d5097efc6 100644 --- a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java +++ b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java @@ -40,7 +40,6 @@ import com.google.cloud.dataflow.sdk.transforms.display.DisplayData.Builder; import com.google.cloud.dataflow.sdk.transforms.display.DisplayData.Item; import com.google.cloud.dataflow.sdk.values.PCollection; -import com.google.common.base.Optional; import com.google.common.testing.EqualsTester; import org.hamcrest.CustomTypeSafeMatcher; @@ -60,7 +59,6 @@ import java.io.IOException; import java.util.Collection; import java.util.Map; -import java.util.Objects; import java.util.regex.Pattern; /** @@ -589,20 +587,18 @@ public void populateDisplayData(Builder builder) { @Test public void testKnownTypeInference() { - assertThat(DisplayData.inferType(1234), hasOptionalValue(DisplayData.Type.INTEGER)); - assertThat(DisplayData.inferType(1234L), hasOptionalValue(DisplayData.Type.INTEGER)); - assertThat(DisplayData.inferType(12.3), hasOptionalValue(DisplayData.Type.FLOAT)); - assertThat(DisplayData.inferType(12.3f), hasOptionalValue(DisplayData.Type.FLOAT)); - assertThat(DisplayData.inferType(true), hasOptionalValue(DisplayData.Type.BOOLEAN)); - assertThat(DisplayData.inferType(Instant.now()), hasOptionalValue(DisplayData.Type.TIMESTAMP)); - assertThat(DisplayData.inferType(Duration.millis(1234)), - hasOptionalValue(DisplayData.Type.DURATION)); - assertThat(DisplayData.inferType(DisplayDataTest.class), - hasOptionalValue(DisplayData.Type.JAVA_CLASS)); - assertThat(DisplayData.inferType("hello world"), hasOptionalValue(DisplayData.Type.STRING)); + assertEquals(DisplayData.Type.INTEGER, DisplayData.inferType(1234)); + assertEquals(DisplayData.Type.INTEGER, DisplayData.inferType(1234L)); + assertEquals(DisplayData.Type.FLOAT, DisplayData.inferType(12.3)); + assertEquals(DisplayData.Type.FLOAT, DisplayData.inferType(12.3f)); + assertEquals(DisplayData.Type.BOOLEAN, DisplayData.inferType(true)); + assertEquals(DisplayData.Type.TIMESTAMP, DisplayData.inferType(Instant.now())); + assertEquals(DisplayData.Type.DURATION, DisplayData.inferType(Duration.millis(1234))); + assertEquals(DisplayData.Type.JAVA_CLASS, DisplayData.inferType(DisplayDataTest.class)); + assertEquals(DisplayData.Type.STRING, DisplayData.inferType("hello world")); - assertEquals(Optional.absent(), DisplayData.inferType(null)); - assertEquals(Optional.absent(), DisplayData.inferType(new Object() {})); + assertEquals(null, DisplayData.inferType(null)); + assertEquals(null, DisplayData.inferType(new Object() {})); } @Test @@ -791,13 +787,4 @@ protected String featureValueOf(DisplayData.Item actual) { } }; } - - private static Matcher> hasOptionalValue(final T value) { - return new CustomTypeSafeMatcher>("optional value") { - @Override - protected boolean matchesSafely(Optional item) { - return item.isPresent() && Objects.equals(item.get(), value); - } - }; - } } From d8312e00c463ad8ef59337734d8954997f14e47a Mon Sep 17 00:00:00 2001 From: Scott Wegner Date: Fri, 8 Apr 2016 09:43:07 -0700 Subject: [PATCH 6/6] fixup: give helper method for extracting DisplayData namespaces a better name --- .../dataflow/sdk/transforms/display/DisplayData.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java b/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java index 8dfabb1061fe..d23fc0b797d4 100644 --- a/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java +++ b/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java @@ -125,8 +125,8 @@ public String toString() { return builder.toString(); } - private static String convertNamespace(Class nsClass) { - return nsClass.getName(); + private static String namespaceOf(Class clazz) { + return clazz.getName(); } /** @@ -339,7 +339,7 @@ public static class Item { private static Item create(Class nsClass, String key, Type type, Object value) { FormattedItemValue formatted = type.format(value); - String namespace = convertNamespace(nsClass); + String namespace = namespaceOf(nsClass); return new Item( namespace, key, type, formatted.getLongValue(), formatted.getShortValue(), null, null); } @@ -470,7 +470,7 @@ private Item withUrl(String url) { } private Item withNamespace(Class nsClass) { - String namespace = convertNamespace(nsClass); + String namespace = namespaceOf(nsClass); return new Item( namespace, this.key, this.type, this.value, this.shortValue, this.url, this.label); } @@ -491,7 +491,7 @@ public static class Identifier { private final String key; public static Identifier of(Class namespace, String key) { - return of(convertNamespace(namespace), key); + return of(namespaceOf(namespace), key); } public static Identifier of(String namespace, String key) {