From efaac0648728e49692f8972a03b122a92994a42e Mon Sep 17 00:00:00 2001 From: cpovirk Date: Wed, 20 Nov 2019 13:39:55 -0800 Subject: [PATCH] Implementation tweaks: 1. Rename keyOrder to actualAndExpectedKeys. The keyOrder name suggests that ordering is significant, when in fact: - the ProtoTruth logic doesn't take order into consideration - map fields do not have defined order: https://developers.google.com/protocol-buffers/docs/proto3#maps 2. Extract a method for retrieving the value of a field, substituting the default value. Use it in toProtoMap(). RELNOTES=n/a ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=281590847 --- .../proto/ProtoTruthMessageDifferencer.java | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/extensions/proto/src/main/java/com/google/common/truth/extensions/proto/ProtoTruthMessageDifferencer.java b/extensions/proto/src/main/java/com/google/common/truth/extensions/proto/ProtoTruthMessageDifferencer.java index 3e70ff9ab..755a583cb 100644 --- a/extensions/proto/src/main/java/com/google/common/truth/extensions/proto/ProtoTruthMessageDifferencer.java +++ b/extensions/proto/src/main/java/com/google/common/truth/extensions/proto/ProtoTruthMessageDifferencer.java @@ -116,14 +116,14 @@ private DiffResult diffMessages(Message actual, Message expected, FluentEquality Map actualMap = toProtoMap(actualFields.get(fieldDescriptor)); Map expectedMap = toProtoMap(expectedFields.get(fieldDescriptor)); - ImmutableSet keyOrder = + ImmutableSet actualAndExpectedKeys = Sets.union(actualMap.keySet(), expectedMap.keySet()).immutableCopy(); builder.addAllSingularFields( fieldDescriptor.getNumber(), compareMapFieldsByKey( actualMap, expectedMap, - keyOrder, + actualAndExpectedKeys, fieldDescriptor, config.subScope(rootDescriptor, fieldDescriptorOrUnknown))); } else { @@ -206,20 +206,17 @@ private static ImmutableMap toProtoMap(@NullableDecl Object cont Map retVal = Maps.newHashMap(); for (Object entry : entryMessages) { Message message = (Message) entry; - Object key = message.getAllFields().get(message.getDescriptorForType().findFieldByNumber(1)); - if (key == null) { - key = message.getDescriptorForType().findFieldByNumber(1).getDefaultValue(); - } - Object value = - message.getAllFields().get(message.getDescriptorForType().findFieldByNumber(2)); - if (value == null) { - value = message.getDescriptorForType().findFieldByNumber(2).getDefaultValue(); - } - retVal.put(key, value); + retVal.put(valueAtFieldNumber(message, 1), valueAtFieldNumber(message, 2)); } return ImmutableMap.copyOf(retVal); } + private static Object valueAtFieldNumber(Message message, int fieldNumber) { + FieldDescriptor field = message.getDescriptorForType().findFieldByNumber(fieldNumber); + Object value = message.getAllFields().get(field); + return value != null ? value : field.getDefaultValue(); + } + // Takes a List or null, and returns the casted list in the first case, an empty list in // the latter case. private static List toProtoList(@NullableDecl Object container) { @@ -232,7 +229,7 @@ private static List toProtoList(@NullableDecl Object container) { private List compareMapFieldsByKey( Map actualMap, Map expectedMap, - Set keyOrder, + Set actualAndExpectedKeys, FieldDescriptor mapFieldDescriptor, FluentEqualityConfig mapConfig) { FieldDescriptor keyFieldDescriptor = mapFieldDescriptor.getMessageType().findFieldByNumber(1); @@ -257,8 +254,8 @@ private List compareMapFieldsByKey( mapConfig.subScope(rootDescriptor, valueFieldDescriptorOrUnknown); ImmutableList.Builder builder = - ImmutableList.builderWithExpectedSize(keyOrder.size()); - for (Object key : keyOrder) { + ImmutableList.builderWithExpectedSize(actualAndExpectedKeys.size()); + for (Object key : actualAndExpectedKeys) { @NullableDecl Object actualValue = actualMap.get(key); @NullableDecl Object expectedValue = expectedMap.get(key); if (ignoreExtraRepeatedFieldElements && !expectedMap.isEmpty() && expectedValue == null) {