Ignore trivial differences when comparing DB (#1567)

* Ignore trivial differences when comparing DB

Some data difference are due to entity model differences and also
harmless. We should igore them when comparing Datastore and SQL.

  This PR ignores the following diffs:
  - null vs empty collection
  - the empty string in Address.stree field, which is a list
This commit is contained in:
Weimin Yu 2022-03-23 13:52:31 -04:00 committed by GitHub
parent eefd197ad5
commit 187432890a
2 changed files with 81 additions and 2 deletions

View file

@ -20,6 +20,8 @@ import static google.registry.persistence.transaction.TransactionManagerFactory.
import com.google.common.base.Preconditions; import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import com.google.common.collect.Maps.EntryTransformer;
import google.registry.beam.initsql.Transforms; import google.registry.beam.initsql.Transforms;
import google.registry.config.RegistryEnvironment; import google.registry.config.RegistryEnvironment;
import google.registry.model.EppResource; import google.registry.model.EppResource;
@ -37,19 +39,24 @@ import google.registry.model.registrar.Registrar;
import google.registry.model.replay.SqlEntity; import google.registry.model.replay.SqlEntity;
import google.registry.model.reporting.HistoryEntry; import google.registry.model.reporting.HistoryEntry;
import google.registry.util.DiffUtils; import google.registry.util.DiffUtils;
import java.io.Serializable;
import java.lang.reflect.Field; import java.lang.reflect.Field;
import java.math.BigInteger; import java.math.BigInteger;
import java.util.Collection;
import java.util.HashMap; import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Objects; import java.util.Objects;
import java.util.Optional; import java.util.Optional;
import java.util.Set; import java.util.Set;
import java.util.stream.Collectors;
import org.apache.beam.sdk.metrics.Counter; import org.apache.beam.sdk.metrics.Counter;
import org.apache.beam.sdk.metrics.Metrics; import org.apache.beam.sdk.metrics.Metrics;
import org.apache.beam.sdk.transforms.DoFn; import org.apache.beam.sdk.transforms.DoFn;
import org.apache.beam.sdk.values.KV; import org.apache.beam.sdk.values.KV;
import org.apache.beam.sdk.values.TupleTag; import org.apache.beam.sdk.values.TupleTag;
import org.checkerframework.checker.nullness.qual.Nullable;
/** Helpers for use by {@link ValidateDatabasePipeline}. */ /** Helpers for use by {@link ValidateDatabasePipeline}. */
@DeleteAfterMigration @DeleteAfterMigration
@ -194,13 +201,52 @@ final class ValidateSqlUtils {
return; return;
} }
if (!Objects.equals(entity0, entity1)) { Map<String, Object> fields0 =
Maps.transformEntries(
((ImmutableObject) entity0).toDiffableFieldMap(), new DiffableFieldNormalizer());
Map<String, Object> fields1 =
Maps.transformEntries(
((ImmutableObject) entity1).toDiffableFieldMap(), new DiffableFieldNormalizer());
if (!Objects.equals(fields0, fields1)) {
unequalCounters.get(counterKey).inc(); unequalCounters.get(counterKey).inc();
out.output(unEqualEntityLog(kv.getKey(), entities.get(0), entities.get(1))); out.output(kv.getKey() + " " + DiffUtils.prettyPrintEntityDeepDiff(fields0, fields1));
} }
} }
} }
/**
* Normalizes trivial differences between objects persisted in Datastore and SQL.
*
* <p>This class works on a map generated by {@link ImmutableObject#toDiffableFieldMap}, and
* performs the following changes:
*
* <ul>
* <li>If a value is an empty {@link Collection}, it is converted to null
* <li>For each {@link google.registry.model.eppcommon.Address} object, empty strings are
* removed from its {@code string} field, which is a {@link List}.
* </ul>
*/
static class DiffableFieldNormalizer
implements EntryTransformer<String, Object, Object>, Serializable {
@Override
public Object transformEntry(String key, @Nullable Object value) {
if (value instanceof Collection && ((Collection<?>) value).isEmpty()) {
return null;
}
if (key.equals("street") && value instanceof List) {
return ((List<?>) value)
.stream().filter(v -> v != null && !Objects.equals("", v)).collect(Collectors.toList());
}
// Short-term hack: LinkedHashMap<String, ?> represents a child ImmutableObject instance.
if (value instanceof LinkedHashMap
&& ((LinkedHashMap<?, ?>) value).keySet().stream().anyMatch(e -> e instanceof String)) {
return Maps.transformEntries((Map<String, Object>) value, this);
}
return value;
}
}
static SqlEntity normalizeEntity(SqlEntity sqlEntity) { static SqlEntity normalizeEntity(SqlEntity sqlEntity) {
if (sqlEntity instanceof EppResource) { if (sqlEntity instanceof EppResource) {
return normalizeEppResource(sqlEntity); return normalizeEppResource(sqlEntity);

View file

@ -19,11 +19,19 @@ import static google.registry.beam.comparedb.ValidateSqlUtils.getMedianIdForHist
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import static org.joda.time.DateTimeZone.UTC; import static org.joda.time.DateTimeZone.UTC;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Maps;
import com.google.common.truth.Truth; import com.google.common.truth.Truth;
import google.registry.beam.comparedb.ValidateSqlUtils.DiffableFieldNormalizer;
import google.registry.model.bulkquery.TestSetupHelper; import google.registry.model.bulkquery.TestSetupHelper;
import google.registry.model.contact.ContactAddress;
import google.registry.model.contact.ContactResource;
import google.registry.model.contact.PostalInfo;
import google.registry.model.domain.DomainHistory; import google.registry.model.domain.DomainHistory;
import google.registry.testing.AppEngineExtension; import google.registry.testing.AppEngineExtension;
import google.registry.testing.FakeClock; import google.registry.testing.FakeClock;
import google.registry.util.DiffUtils;
import java.util.Map;
import org.joda.time.DateTime; import org.joda.time.DateTime;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.api.extension.RegisterExtension;
@ -60,4 +68,29 @@ class ValidateSqlUtilsTest {
assertThat(getMedianIdForHistoryTable("DomainHistory")) assertThat(getMedianIdForHistoryTable("DomainHistory"))
.hasValue(setupHelper.domainHistory.getId()); .hasValue(setupHelper.domainHistory.getId());
} }
@Test
void diffableFieldNormalizer() {
ContactResource contactResource =
new ContactResource.Builder()
.setLocalizedPostalInfo(
new PostalInfo.Builder()
.setType(PostalInfo.Type.LOCALIZED)
.setAddress(
new ContactAddress.Builder()
.setStreet(ImmutableList.of("111 8th Ave", ""))
.setCity("New York")
.setState("NY")
.setZip("10011")
.setCountryCode("US")
.build())
.build())
.build();
Map<String, Object> origMap = contactResource.toDiffableFieldMap();
Map<String, Object> trimmedMap = Maps.transformEntries(origMap, new DiffableFieldNormalizer());
// In the trimmed map, localizedPostalInfo.address.street only has one element in the list,
// thus the output: 'null -> '
Truth.assertThat(DiffUtils.prettyPrintEntityDeepDiff(trimmedMap, origMap))
.isEqualTo("localizedPostalInfo.address.street.1: null -> \n");
}
} }