Persist *History objects as HistoryEntry objects (#749)

* Persist *History objects as HistoryEntry objects

While Datastore is the primary database, we will store *History objects
as HistoryEntry objects and convert to/from the proper objects in the
Datastore transaction manager. This means that History objects will not
properly store the copy of the EppResource until we move to SQL as
primary, but this is the way the world exists anyway so it's not a
problem.

* Format code and simplify the bulk loading

* Add comments with context
This commit is contained in:
gbrodman
2020-08-26 11:45:09 -04:00
committed by GitHub
parent df15b38a1e
commit 266bd43792
10 changed files with 401 additions and 54 deletions
@@ -19,9 +19,12 @@ import google.registry.model.billing.BillingEvent;
import google.registry.model.common.Cursor;
import google.registry.model.common.EntityGroupRoot;
import google.registry.model.common.GaeUserIdConverter;
import google.registry.model.contact.ContactHistory;
import google.registry.model.contact.ContactResource;
import google.registry.model.domain.DomainBase;
import google.registry.model.domain.DomainHistory;
import google.registry.model.domain.token.AllocationToken;
import google.registry.model.host.HostHistory;
import google.registry.model.host.HostResource;
import google.registry.model.index.EppResourceIndex;
import google.registry.model.index.EppResourceIndexBucket;
@@ -68,9 +71,11 @@ public final class EntityClasses {
CommitLogCheckpointRoot.class,
CommitLogManifest.class,
CommitLogMutation.class,
ContactHistory.class,
ContactResource.class,
Cursor.class,
DomainBase.class,
DomainHistory.class,
EntityGroupRoot.class,
EppResourceIndex.class,
EppResourceIndexBucket.class,
@@ -79,6 +84,7 @@ public final class EntityClasses {
ForeignKeyIndex.ForeignKeyHostIndex.class,
GaeUserIdConverter.class,
HistoryEntry.class,
HostHistory.class,
HostResource.class,
KmsSecret.class,
KmsSecretRevision.class,
@@ -15,6 +15,7 @@
package google.registry.model.contact;
import com.googlecode.objectify.Key;
import com.googlecode.objectify.annotation.EntitySubclass;
import google.registry.model.EppResource;
import google.registry.model.reporting.HistoryEntry;
import google.registry.persistence.VKey;
@@ -36,6 +37,7 @@ import javax.persistence.Entity;
@javax.persistence.Index(columnList = "historyType"),
@javax.persistence.Index(columnList = "historyModificationTime")
})
@EntitySubclass
public class ContactHistory extends HistoryEntry {
// Store ContactBase instead of ContactResource so we don't pick up its @Id
ContactBase contactBase;
@@ -15,6 +15,7 @@
package google.registry.model.domain;
import com.googlecode.objectify.Key;
import com.googlecode.objectify.annotation.EntitySubclass;
import google.registry.model.EppResource;
import google.registry.model.contact.ContactResource;
import google.registry.model.host.HostResource;
@@ -43,6 +44,7 @@ import javax.persistence.JoinTable;
@javax.persistence.Index(columnList = "historyType"),
@javax.persistence.Index(columnList = "historyModificationTime")
})
@EntitySubclass
public class DomainHistory extends HistoryEntry {
// Store DomainContent instead of DomainBase so we don't pick up its @Id
DomainContent domainContent;
@@ -15,6 +15,7 @@
package google.registry.model.host;
import com.googlecode.objectify.Key;
import com.googlecode.objectify.annotation.EntitySubclass;
import google.registry.model.EppResource;
import google.registry.model.reporting.HistoryEntry;
import google.registry.persistence.VKey;
@@ -37,6 +38,7 @@ import javax.persistence.Entity;
@javax.persistence.Index(columnList = "historyType"),
@javax.persistence.Index(columnList = "historyModificationTime")
})
@EntitySubclass
public class HostHistory extends HistoryEntry {
// Store HostBase instead of HostResource so we don't pick up its @Id
@@ -24,13 +24,16 @@ import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.googlecode.objectify.Key;
import google.registry.model.contact.ContactHistory;
import google.registry.model.host.HostHistory;
import google.registry.model.reporting.HistoryEntry;
import google.registry.persistence.VKey;
import google.registry.persistence.transaction.TransactionManager;
import java.util.Map.Entry;
import java.util.NoSuchElementException;
import java.util.Optional;
import java.util.function.Supplier;
import java.util.stream.StreamSupport;
import javax.annotation.Nullable;
import org.joda.time.DateTime;
/** Datastore implementation of {@link TransactionManager}. */
@@ -99,8 +102,7 @@ public class DatastoreTransactionManager implements TransactionManager {
@Override
public void saveNew(Object entity) {
checkArgumentNotNull(entity, "entity must be specified");
getOfy().save().entity(entity);
saveEntity(entity);
}
@Override
@@ -110,8 +112,7 @@ public class DatastoreTransactionManager implements TransactionManager {
@Override
public void saveNewOrUpdate(Object entity) {
checkArgumentNotNull(entity, "entity must be specified");
getOfy().save().entity(entity);
saveEntity(entity);
}
@Override
@@ -121,8 +122,7 @@ public class DatastoreTransactionManager implements TransactionManager {
@Override
public void update(Object entity) {
checkArgumentNotNull(entity, "entity must be specified");
getOfy().save().entity(entity);
saveEntity(entity);
}
@Override
@@ -137,7 +137,7 @@ public class DatastoreTransactionManager implements TransactionManager {
@Override
public <T> boolean checkExists(VKey<T> key) {
return getOfy().load().key(key.getOfyKey()).now() != null;
return loadNullable(key) != null;
}
// TODO: add tests for these methods. They currently have some degree of test coverage because
@@ -146,12 +146,12 @@ public class DatastoreTransactionManager implements TransactionManager {
// interface tests that are applied to both the datastore and SQL implementations.
@Override
public <T> Optional<T> maybeLoad(VKey<T> key) {
return Optional.ofNullable(getOfy().load().key(key.getOfyKey()).now());
return Optional.ofNullable(loadNullable(key));
}
@Override
public <T> T load(VKey<T> key) {
T result = getOfy().load().key(key.getOfyKey()).now();
T result = loadNullable(key);
if (result == null) {
throw new NoSuchElementException(key.toString());
}
@@ -167,7 +167,10 @@ public class DatastoreTransactionManager implements TransactionManager {
.collect(toImmutableMap(key -> (Key<T>) key.getOfyKey(), Functions.identity()));
return getOfy().load().keys(keyMap.keySet()).entrySet().stream()
.collect(ImmutableMap.toImmutableMap(entry -> keyMap.get(entry.getKey()), Entry::getValue));
.collect(
toImmutableMap(
entry -> keyMap.get(entry.getKey()),
entry -> toChildHistoryEntryIfPossible(entry.getValue())));
}
@Override
@@ -191,4 +194,37 @@ public class DatastoreTransactionManager implements TransactionManager {
.collect(toImmutableList());
getOfy().delete().keys(list).now();
}
/**
* The following three methods exist due to the migration to Cloud SQL.
*
* <p>In Cloud SQL, {@link HistoryEntry} objects are represented instead as {@link DomainHistory},
* {@link ContactHistory}, and {@link HostHistory} objects. During the migration, we do not wish
* to change the Datastore schema so all of these objects are stored in Datastore as HistoryEntry
* objects. They are converted to/from the appropriate classes upon retrieval, and converted to
* HistoryEntry on save. See go/r3.0-history-objects for more details.
*/
private void saveEntity(Object entity) {
checkArgumentNotNull(entity, "entity must be specified");
if (entity instanceof HistoryEntry) {
entity = ((HistoryEntry) entity).asHistoryEntry();
}
getOfy().save().entity(entity);
}
@SuppressWarnings("unchecked")
private <T> T toChildHistoryEntryIfPossible(@Nullable T obj) {
// NB: The Key of the object in question may not necessarily be the resulting class that we
// wish to have. Because all *History classes are @EntitySubclasses, their Keys will have type
// HistoryEntry -- even if you create them based off the *History class.
if (obj != null && HistoryEntry.class.isAssignableFrom(obj.getClass())) {
return (T) ((HistoryEntry) obj).toChildHistoryEntity();
}
return obj;
}
@Nullable
private <T> T loadNullable(VKey<T> key) {
return toChildHistoryEntryIfPossible(getOfy().load().key(key.getOfyKey()).now());
}
}
@@ -14,8 +14,10 @@
package google.registry.model.reporting;
import static com.googlecode.objectify.Key.getKind;
import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableSet;
import com.googlecode.objectify.Key;
import com.googlecode.objectify.annotation.Entity;
@@ -28,8 +30,14 @@ import google.registry.model.Buildable;
import google.registry.model.EppResource;
import google.registry.model.ImmutableObject;
import google.registry.model.annotations.ReportedOn;
import google.registry.model.contact.ContactHistory;
import google.registry.model.contact.ContactResource;
import google.registry.model.domain.DomainBase;
import google.registry.model.domain.DomainHistory;
import google.registry.model.domain.Period;
import google.registry.model.eppcommon.Trid;
import google.registry.model.host.HostHistory;
import google.registry.model.host.HostResource;
import google.registry.persistence.VKey;
import google.registry.persistence.WithStringVKey;
import java.util.Set;
@@ -111,7 +119,8 @@ public class HistoryEntry extends ImmutableObject implements Buildable {
@Id
@javax.persistence.Id
@Column(name = "historyRevisionId")
Long id;
@VisibleForTesting
public Long id;
/** The resource this event mutated. */
@Parent @Transient protected Key<? extends EppResource> parent;
@@ -259,6 +268,39 @@ public class HistoryEntry extends ImmutableObject implements Buildable {
return new Builder(clone(this));
}
public HistoryEntry asHistoryEntry() {
return new Builder().copyFrom(this).build();
}
public HistoryEntry toChildHistoryEntity() {
String parentKind = getParent().getKind();
final HistoryEntry resultEntity;
// can't use a switch statement since we're calling getKind()
if (parentKind.equals(getKind(DomainBase.class))) {
resultEntity =
new DomainHistory.Builder()
.copyFrom(this)
.setDomainRepoId(VKey.create(DomainBase.class, parent.getName(), parent))
.build();
} else if (parentKind.equals(getKind(HostResource.class))) {
resultEntity =
new HostHistory.Builder()
.copyFrom(this)
.setHostRepoId(VKey.create(HostResource.class, parent.getName(), parent))
.build();
} else if (parentKind.equals(getKind(ContactResource.class))) {
resultEntity =
new ContactHistory.Builder()
.copyFrom(this)
.setContactRepoId(VKey.create(ContactResource.class, parent.getName(), parent))
.build();
} else {
throw new IllegalStateException(
String.format("Unknown kind of HistoryEntry parent %s", parentKind));
}
return resultEntity;
}
/** A builder for {@link HistoryEntry} since it is immutable */
public static class Builder<T extends HistoryEntry, B extends Builder<?, ?>>
extends GenericBuilder<T, B> {
@@ -268,17 +310,40 @@ public class HistoryEntry extends ImmutableObject implements Buildable {
super(instance);
}
// Used to fill out the fields in this object from an object which may not be exactly the same
// as the class T, where both classes still subclass HistoryEntry
public B copyFrom(HistoryEntry historyEntry) {
setId(historyEntry.id);
setParent(historyEntry.parent);
setType(historyEntry.type);
setPeriod(historyEntry.period);
setXmlBytes(historyEntry.xmlBytes);
setModificationTime(historyEntry.modificationTime);
setClientId(historyEntry.clientId);
setOtherClientId(historyEntry.otherClientId);
setTrid(historyEntry.trid);
setBySuperuser(historyEntry.bySuperuser);
setReason(historyEntry.reason);
setRequestedByRegistrar(historyEntry.requestedByRegistrar);
setDomainTransactionRecords(nullToEmptyImmutableCopy(historyEntry.domainTransactionRecords));
return thisCastToDerived();
}
@Override
public T build() {
return super.build();
}
public B setId(long id) {
getInstance().id = id;
return thisCastToDerived();
}
public B setParent(EppResource parent) {
getInstance().parent = Key.create(parent);
return thisCastToDerived();
}
// Until we move completely to SQL, override this in subclasses (e.g. HostHistory) to set VKeys
public B setParent(Key<? extends EppResource> parent) {
getInstance().parent = parent;
return thisCastToDerived();