Skip to content

Commit

Permalink
Implementation tweaks:
Browse files Browse the repository at this point in the history
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
  • Loading branch information
cpovirk authored and nick-someone committed Nov 21, 2019
1 parent 8ebfe2a commit efaac06
Showing 1 changed file with 12 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,14 @@ private DiffResult diffMessages(Message actual, Message expected, FluentEquality
Map<Object, Object> actualMap = toProtoMap(actualFields.get(fieldDescriptor));
Map<Object, Object> expectedMap = toProtoMap(expectedFields.get(fieldDescriptor));

ImmutableSet<Object> keyOrder =
ImmutableSet<Object> actualAndExpectedKeys =
Sets.union(actualMap.keySet(), expectedMap.keySet()).immutableCopy();
builder.addAllSingularFields(
fieldDescriptor.getNumber(),
compareMapFieldsByKey(
actualMap,
expectedMap,
keyOrder,
actualAndExpectedKeys,
fieldDescriptor,
config.subScope(rootDescriptor, fieldDescriptorOrUnknown)));
} else {
Expand Down Expand Up @@ -206,20 +206,17 @@ private static ImmutableMap<Object, Object> toProtoMap(@NullableDecl Object cont
Map<Object, Object> 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<Object> 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) {
Expand All @@ -232,7 +229,7 @@ private static List<?> toProtoList(@NullableDecl Object container) {
private List<SingularField> compareMapFieldsByKey(
Map<Object, Object> actualMap,
Map<Object, Object> expectedMap,
Set<Object> keyOrder,
Set<Object> actualAndExpectedKeys,
FieldDescriptor mapFieldDescriptor,
FluentEqualityConfig mapConfig) {
FieldDescriptor keyFieldDescriptor = mapFieldDescriptor.getMessageType().findFieldByNumber(1);
Expand All @@ -257,8 +254,8 @@ private List<SingularField> compareMapFieldsByKey(
mapConfig.subScope(rootDescriptor, valueFieldDescriptorOrUnknown);

ImmutableList.Builder<SingularField> 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) {
Expand Down

0 comments on commit efaac06

Please sign in to comment.