Skip to content

Commit dba82fe

Browse files
swegnerbchambers
swegner
authored and
bchambers
committed
DisplayData API tweaks
- Add additional overloads for conditionally registering null display data - Serialize DisplayData using JSON primatives - Fix checkstyle error - Bind generic type parameter in DataflowPipelineTranslator#addDisplayData - Add additional tests for new APIs - Improve readability of DisplayDataTest#populateDisplayData
1 parent 920f14d commit dba82fe

5 files changed

Lines changed: 275 additions & 60 deletions

File tree

‎runners/google-cloud-dataflow-java/src/main/java/com/google/cloud/dataflow/sdk/runners/DataflowPipelineTranslator.java‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@
8181
import com.google.cloud.dataflow.sdk.values.TypedPValue;
8282
import com.google.common.base.Preconditions;
8383
import com.google.common.base.Strings;
84-
import com.google.common.collect.Lists;
8584

8685
import com.fasterxml.jackson.core.JsonProcessingException;
8786
import com.fasterxml.jackson.databind.ObjectMapper;
@@ -730,12 +729,8 @@ private void addOutput(String name, PValue value, Coder<?> valueCoder) {
730729
}
731730

732731
private void addDisplayData(String name, DisplayData displayData) {
733-
List<Map<String, Object>> serializedItems = Lists.newArrayList();
734-
for (DisplayData.Item item : displayData.items()) {
735-
serializedItems.add(MAPPER.convertValue(item, Map.class));
736-
}
737-
738-
addList(getProperties(), name, serializedItems);
732+
List<Map<String, Object>> list = MAPPER.convertValue(displayData, List.class);
733+
addList(getProperties(), name, list);
739734
}
740735

741736
@Override

‎runners/google-cloud-dataflow-java/src/test/java/com/google/cloud/dataflow/sdk/runners/DataflowPipelineTranslatorTest.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -818,8 +818,8 @@ public void processElement(ProcessContext c) throws Exception {
818818
@Override
819819
public void populateDisplayData(DisplayData.Builder builder) {
820820
builder
821-
.add("foo", "bar")
822-
.add("foo2", DataflowPipelineTranslatorTest.class)
821+
.add("foo", "bar")
822+
.add("foo2", DataflowPipelineTranslatorTest.class)
823823
.withLabel("Test Class")
824824
.withLinkUrl("http://www.google.com");
825825
}
@@ -833,7 +833,7 @@ public void processElement(ProcessContext c) throws Exception {
833833

834834
@Override
835835
public void populateDisplayData(DisplayData.Builder builder) {
836-
builder.add("foo3", "barge");
836+
builder.add("foo3", 1234);
837837
}
838838
};
839839

@@ -876,11 +876,11 @@ public void populateDisplayData(DisplayData.Builder builder) {
876876
);
877877

878878
ImmutableList expectedFn2DisplayData = ImmutableList.of(
879-
ImmutableMap.<String, String>builder()
879+
ImmutableMap.<String, Object>builder()
880880
.put("namespace", fn2.getClass().getName())
881881
.put("key", "foo3")
882-
.put("type", "STRING")
883-
.put("value", "barge")
882+
.put("type", "INTEGER")
883+
.put("value", 1234L)
884884
.build()
885885
);
886886

‎sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java‎

Lines changed: 79 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929

3030
import com.fasterxml.jackson.annotation.JsonGetter;
3131
import com.fasterxml.jackson.annotation.JsonInclude;
32+
import com.fasterxml.jackson.annotation.JsonValue;
3233

3334
import org.apache.avro.reflect.Nullable;
3435
import org.joda.time.Duration;
@@ -100,6 +101,7 @@ public static Type inferType(@Nullable Object value) {
100101
return Type.tryInferFrom(value);
101102
}
102103

104+
@JsonValue
103105
public Collection<Item> items() {
104106
return entries.values();
105107
}
@@ -175,6 +177,13 @@ public interface Builder {
175177
*/
176178
ItemBuilder add(String key, long value);
177179

180+
/**
181+
* Register the given numeric display data if the value is not null.
182+
*
183+
* @see DisplayData.Builder#add(String, long)
184+
*/
185+
ItemBuilder addIfNotNull(String key, @Nullable Long value);
186+
178187
/**
179188
* Register the given numeric display data if the value is different than the specified default.
180189
*
@@ -189,6 +198,13 @@ public interface Builder {
189198
*/
190199
ItemBuilder add(String key, double value);
191200

201+
/**
202+
* Register the given floating point display data if the value is not null.
203+
*
204+
* @see DisplayData.Builder#add(String, double)
205+
*/
206+
ItemBuilder addIfNotNull(String key, @Nullable Double value);
207+
192208
/**
193209
* Register the given floating point display data if the value is different than the specified
194210
* default.
@@ -204,6 +220,13 @@ public interface Builder {
204220
*/
205221
ItemBuilder add(String key, boolean value);
206222

223+
/**
224+
* Register the given boolean display data if the value is not null.
225+
*
226+
* @see DisplayData.Builder#add(String, boolean)
227+
*/
228+
ItemBuilder addIfNotNull(String key, @Nullable Boolean value);
229+
207230
/**
208231
* Register the given boolean display data if the value is different than the specified default.
209232
*
@@ -286,6 +309,7 @@ ItemBuilder addIfNotDefault(
286309
* transform or component.
287310
*
288311
* @throws ClassCastException if the value cannot be safely cast to the specified type.
312+
*
289313
* @see DisplayData#inferType(Object)
290314
*/
291315
ItemBuilder add(String key, Type type, Object value);
@@ -332,8 +356,8 @@ public static class Item {
332356
private final String key;
333357
private final String ns;
334358
private final Type type;
335-
private final String value;
336-
private final String shortValue;
359+
private final Object value;
360+
private final Object shortValue;
337361
private final String label;
338362
private final String url;
339363

@@ -348,8 +372,8 @@ private Item(
348372
String namespace,
349373
String key,
350374
Type type,
351-
String value,
352-
String shortValue,
375+
Object value,
376+
Object shortValue,
353377
String url,
354378
String label) {
355379
this.ns = namespace;
@@ -384,7 +408,7 @@ public Type getType() {
384408
* Retrieve the value of the metadata item.
385409
*/
386410
@JsonGetter("value")
387-
public String getValue() {
411+
public Object getValue() {
388412
return value;
389413
}
390414

@@ -398,7 +422,7 @@ public String getValue() {
398422
@JsonGetter("shortValue")
399423
@JsonInclude(JsonInclude.Include.NON_NULL)
400424
@Nullable
401-
public String getShortValue() {
425+
public Object getShortValue() {
402426
return shortValue;
403427
}
404428

@@ -540,48 +564,65 @@ public enum Type {
540564
STRING {
541565
@Override
542566
FormattedItemValue format(Object value) {
543-
return new FormattedItemValue(value.toString());
567+
return new FormattedItemValue(checkType(value, String.class, STRING));
544568
}
545569
},
546570
INTEGER {
547571
@Override
548572
FormattedItemValue format(Object value) {
549-
Number number = (Number) value;
550-
return new FormattedItemValue(Long.toString(number.longValue()));
573+
if (value instanceof Integer) {
574+
long l = ((Integer) value).longValue();
575+
return format(l);
576+
}
577+
578+
return new FormattedItemValue(checkType(value, Long.class, INTEGER));
551579
}
552580
},
553581
FLOAT {
554582
@Override
555583
FormattedItemValue format(Object value) {
556-
return new FormattedItemValue(Double.toString((Double) value));
584+
return new FormattedItemValue(checkType(value, Number.class, FLOAT));
557585
}
558586
},
559587
BOOLEAN() {
560588
@Override
561589
FormattedItemValue format(Object value) {
562-
return new FormattedItemValue(Boolean.toString((boolean) value));
590+
return new FormattedItemValue(checkType(value, Boolean.class, BOOLEAN));
563591
}
564592
},
565593
TIMESTAMP() {
566594
@Override
567595
FormattedItemValue format(Object value) {
568-
return new FormattedItemValue((TIMESTAMP_FORMATTER.print((Instant) value)));
596+
Instant instant = checkType(value, Instant.class, TIMESTAMP);
597+
return new FormattedItemValue((TIMESTAMP_FORMATTER.print(instant)));
569598
}
570599
},
571600
DURATION {
572601
@Override
573602
FormattedItemValue format(Object value) {
574-
return new FormattedItemValue(Long.toString(((Duration) value).getMillis()));
603+
Duration duration = checkType(value, Duration.class, DURATION);
604+
return new FormattedItemValue(duration.getMillis());
575605
}
576606
},
577607
JAVA_CLASS {
578608
@Override
579609
FormattedItemValue format(Object value) {
580-
Class<?> clazz = (Class<?>) value;
610+
Class<?> clazz = checkType(value, Class.class, JAVA_CLASS);
581611
return new FormattedItemValue(clazz.getName(), clazz.getSimpleName());
582612
}
583613
};
584614

615+
private static <T> T checkType(Object value, Class<T> clazz, DisplayData.Type expectedType) {
616+
if (!clazz.isAssignableFrom(value.getClass())) {
617+
throw new ClassCastException(String.format(
618+
"Value is not valid for DisplayData type %s: %s", expectedType, value));
619+
}
620+
621+
@SuppressWarnings("unchecked") // type checked above.
622+
T typedValue = (T) value;
623+
return typedValue;
624+
}
625+
585626
/**
586627
* Format the display metadata value into a long string representation, and optionally
587628
* a shorter representation for display.
@@ -592,7 +633,6 @@ FormattedItemValue format(Object value) {
592633

593634
@Nullable
594635
private static Type tryInferFrom(@Nullable Object value) {
595-
Type type;
596636
if (value instanceof Integer || value instanceof Long) {
597637
return INTEGER;
598638
} else if (value instanceof Double || value instanceof Float) {
@@ -614,23 +654,23 @@ private static Type tryInferFrom(@Nullable Object value) {
614654
}
615655

616656
static class FormattedItemValue {
617-
private final String shortValue;
618-
private final String longValue;
657+
private final Object shortValue;
658+
private final Object longValue;
619659

620-
private FormattedItemValue(String longValue) {
660+
private FormattedItemValue(Object longValue) {
621661
this(longValue, null);
622662
}
623663

624-
private FormattedItemValue(String longValue, String shortValue) {
664+
private FormattedItemValue(Object longValue, Object shortValue) {
625665
this.longValue = longValue;
626666
this.shortValue = shortValue;
627667
}
628668

629-
String getLongValue() {
669+
Object getLongValue() {
630670
return this.longValue;
631671
}
632672

633-
String getShortValue() {
673+
Object getShortValue() {
634674
return this.shortValue;
635675
}
636676
}
@@ -700,29 +740,44 @@ public ItemBuilder add(String key, long value) {
700740
return addItemIf(true, key, Type.INTEGER, value);
701741
}
702742

743+
@Override
744+
public ItemBuilder addIfNotNull(String key, @Nullable Long value) {
745+
return addItemIf(value != null, key, Type.INTEGER, value);
746+
}
747+
703748
@Override
704749
public ItemBuilder addIfNotDefault(String key, long value, long defaultValue) {
705-
return addItemIf(value != defaultValue, key, Type.INTEGER, value);
750+
return addItemIf(!Objects.equals(value, defaultValue), key, Type.INTEGER, value);
706751
}
707752

708753
@Override
709754
public ItemBuilder add(String key, double value) {
710755
return addItemIf(true, key, Type.FLOAT, value);
711756
}
712757

758+
@Override
759+
public ItemBuilder addIfNotNull(String key, @Nullable Double value) {
760+
return addItemIf(value != null, key, Type.FLOAT, value);
761+
}
762+
713763
@Override
714764
public ItemBuilder addIfNotDefault(String key, double value, double defaultValue) {
715-
return addItemIf(value != defaultValue, key, Type.FLOAT, value);
765+
return addItemIf(!Objects.equals(value, defaultValue), key, Type.FLOAT, value);
716766
}
717767

718768
@Override
719769
public ItemBuilder add(String key, boolean value) {
720770
return addItemIf(true, key, Type.BOOLEAN, value);
721771
}
722772

773+
@Override
774+
public ItemBuilder addIfNotNull(String key, @Nullable Boolean value) {
775+
return addItemIf(value != null, key, Type.BOOLEAN, value);
776+
}
777+
723778
@Override
724779
public ItemBuilder addIfNotDefault(String key, boolean value, boolean defaultValue) {
725-
return addItemIf(value != defaultValue, key, Type.BOOLEAN, value);
780+
return addItemIf(!Objects.equals(value, defaultValue), key, Type.BOOLEAN, value);
726781
}
727782

728783
@Override

‎sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataMatchers.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -301,20 +301,20 @@ protected DisplayData.Type featureValueOf(DisplayData.Item actual) {
301301
* value.
302302
*/
303303

304-
public static Matcher<DisplayData.Item> hasValue(String value) {
304+
public static Matcher<DisplayData.Item> hasValue(Object value) {
305305
return hasValue(Matchers.is(value));
306306
}
307307

308308
/**
309309
* Creates a matcher that matches if the examined {@link DisplayData.Item} contains a value
310310
* matching the specified value matcher.
311311
*/
312-
public static Matcher<DisplayData.Item> hasValue(Matcher<String> valueMatcher) {
313-
return new FeatureMatcher<DisplayData.Item, String>(
312+
public static <T> Matcher<DisplayData.Item> hasValue(Matcher<T> valueMatcher) {
313+
return new FeatureMatcher<DisplayData.Item, T>(
314314
valueMatcher, "with value", "value") {
315315
@Override
316-
protected String featureValueOf(DisplayData.Item actual) {
317-
return actual.getValue();
316+
protected T featureValueOf(DisplayData.Item actual) {
317+
return (T) actual.getValue();
318318
}
319319
};
320320
}

0 commit comments

Comments
 (0)