Convert more ofy() to auditedOfy() calls (#1152)

A couple of these use the QueryComposer interface to avoid branching.

In addition, we enforce the Datastore restriction that there can be at
most 1 field with an inequality query, see https://cloud.google.com/appengine/docs/standard/go111/datastore/query-restrictions#inequality_filters_are_limited_to_at_most_one_property
This commit is contained in:
gbrodman 2021-05-12 15:06:19 -04:00 committed by GitHub
parent 26ca60617c
commit d0ba4477dd
20 changed files with 118 additions and 83 deletions

View file

@ -14,6 +14,7 @@
package google.registry.model.ofy; package google.registry.model.ofy;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.ImmutableSet.toImmutableSet;
@ -401,11 +402,13 @@ public class DatastoreTransactionManager implements TransactionManager {
} }
private static class DatastoreQueryComposerImpl<T> extends QueryComposer<T> { private static class DatastoreQueryComposerImpl<T> extends QueryComposer<T> {
DatastoreQueryComposerImpl(Class<T> entityClass) { DatastoreQueryComposerImpl(Class<T> entityClass) {
super(entityClass); super(entityClass);
} }
Query<T> buildQuery() { Query<T> buildQuery() {
checkOnlyOneInequalityField();
Query<T> result = ofy().load().type(entityClass); Query<T> result = ofy().load().type(entityClass);
for (WhereClause pred : predicates) { for (WhereClause pred : predicates) {
result = result.filter(pred.fieldName + pred.comparator.getDatastoreString(), pred.value); result = result.filter(pred.fieldName + pred.comparator.getDatastoreString(), pred.value);
@ -420,7 +423,7 @@ public class DatastoreTransactionManager implements TransactionManager {
@Override @Override
public Optional<T> first() { public Optional<T> first() {
return Optional.ofNullable(buildQuery().first().now()); return Optional.ofNullable(buildQuery().limit(1).first().now());
} }
@Override @Override
@ -449,5 +452,20 @@ public class DatastoreTransactionManager implements TransactionManager {
public List<T> list() { public List<T> list() {
return buildQuery().list(); return buildQuery().list();
} }
private void checkOnlyOneInequalityField() {
// Datastore inequality queries are limited to one property, see
// https://cloud.google.com/appengine/docs/standard/go111/datastore/query-restrictions#inequality_filters_are_limited_to_at_most_one_property
long numInequalityFields =
predicates.stream()
.filter(pred -> !pred.comparator.equals(Comparator.EQ))
.map(pred -> pred.fieldName)
.distinct()
.count();
checkArgument(
numInequalityFields <= 1,
"Datastore cannot handle inequality queries on multiple fields, we found %s fields.",
numInequalityFields);
}
} }
} }

View file

@ -69,7 +69,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
// The entity of classes in this set will be simply ignored when passed to modification // The entity of classes in this set will be simply ignored when passed to modification
// operations, i.e. insert, put, update and delete. This is to help maintain a single code path // operations, i.e. insert, put, update and delete. This is to help maintain a single code path
// when we switch from ofy() to tm() for the database migration as we don't need have a condition // when we switch from ofy to tm() for the database migration as we don't need have a condition
// to exclude the Datastore specific entities when the underlying tm() is jpaTm(). // to exclude the Datastore specific entities when the underlying tm() is jpaTm().
// TODO(b/176108270): Remove this property after database migration. // TODO(b/176108270): Remove this property after database migration.
private static final ImmutableSet<Class<? extends ImmutableObject>> IGNORED_ENTITY_CLASSES = private static final ImmutableSet<Class<? extends ImmutableObject>> IGNORED_ENTITY_CLASSES =

View file

@ -17,7 +17,7 @@ package google.registry.tools;
import static com.google.common.base.Strings.isNullOrEmpty; import static com.google.common.base.Strings.isNullOrEmpty;
import static google.registry.flows.poll.PollFlowUtils.SQL_POLL_MESSAGE_QUERY; import static google.registry.flows.poll.PollFlowUtils.SQL_POLL_MESSAGE_QUERY;
import static google.registry.flows.poll.PollFlowUtils.datastorePollMessageQuery; import static google.registry.flows.poll.PollFlowUtils.datastorePollMessageQuery;
import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.ObjectifyService.auditedOfy;
import static google.registry.model.poll.PollMessageExternalKeyConverter.makePollMessageExternalId; import static google.registry.model.poll.PollMessageExternalKeyConverter.makePollMessageExternalId;
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
@ -107,7 +107,7 @@ final class AckPollMessagesCommand implements CommandWithRemoteApi {
tm().transact( tm().transact(
() -> () ->
// Load poll messages and filter to just those of interest. // Load poll messages and filter to just those of interest.
ofy().load().keys(keys).values().stream() auditedOfy().load().keys(keys).values().stream()
.filter(pm -> isNullOrEmpty(message) || pm.getMsg().contains(message)) .filter(pm -> isNullOrEmpty(message) || pm.getMsg().contains(message))
.forEach(this::actOnPollMessage)); .forEach(this::actOnPollMessage));
} }

View file

@ -16,7 +16,7 @@ package google.registry.tools;
import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static google.registry.model.common.EntityGroupRoot.getCrossTldKey; import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.ObjectifyService.auditedOfy;
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import com.beust.jcommander.Parameters; import com.beust.jcommander.Parameters;
@ -37,7 +37,7 @@ final class CompareReservedListsCommand implements CommandWithRemoteApi {
@Override @Override
public void run() { public void run() {
ImmutableSet<String> datastoreLists = ImmutableSet<String> datastoreLists =
ofy().load().type(ReservedList.class).ancestor(getCrossTldKey()).list().stream() auditedOfy().load().type(ReservedList.class).ancestor(getCrossTldKey()).list().stream()
.map(ReservedList::getName) .map(ReservedList::getName)
.collect(toImmutableSet()); .collect(toImmutableSet());

View file

@ -15,7 +15,7 @@
package google.registry.tools; package google.registry.tools;
import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.ObjectifyService.auditedOfy;
import com.beust.jcommander.Parameters; import com.beust.jcommander.Parameters;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
@ -61,9 +61,9 @@ public class DedupeRecurringBillingEventIdsCommand extends ReadEntityFromKeyPath
// Loads the associated DomainBase and BillingEvent.OneTime entities that // Loads the associated DomainBase and BillingEvent.OneTime entities that
// may have reference to this BillingEvent.Recurring entity. // may have reference to this BillingEvent.Recurring entity.
Key<DomainBase> domainKey = getGrandParentAsDomain(Key.create(recurring)); Key<DomainBase> domainKey = getGrandParentAsDomain(Key.create(recurring));
DomainBase domain = ofy().load().key(domainKey).now(); DomainBase domain = auditedOfy().load().key(domainKey).now();
List<BillingEvent.OneTime> oneTimes = List<BillingEvent.OneTime> oneTimes =
ofy().load().type(BillingEvent.OneTime.class).ancestor(domainKey).list(); auditedOfy().load().type(BillingEvent.OneTime.class).ancestor(domainKey).list();
VKey<Recurring> oldRecurringVKey = recurring.createVKey(); VKey<Recurring> oldRecurringVKey = recurring.createVKey();
// By setting id to 0L, Buildable.build() will assign an application wide unique id to it. // By setting id to 0L, Buildable.build() will assign an application wide unique id to it.

View file

@ -15,9 +15,8 @@
package google.registry.tools; package google.registry.tools;
import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Preconditions.checkState;
import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm;
import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameter;
import com.beust.jcommander.Parameters; import com.beust.jcommander.Parameters;
@ -25,6 +24,7 @@ import google.registry.model.domain.DomainBase;
import google.registry.model.registrar.Registrar; import google.registry.model.registrar.Registrar;
import google.registry.model.registry.Registry; import google.registry.model.registry.Registry;
import google.registry.model.registry.Registry.TldType; import google.registry.model.registry.Registry.TldType;
import google.registry.persistence.transaction.QueryComposer.Comparator;
/** /**
* Command to delete the {@link Registry} associated with the specified TLD in Datastore. * Command to delete the {@link Registry} associated with the specified TLD in Datastore.
@ -78,19 +78,11 @@ final class DeleteTldCommand extends ConfirmingCommand implements CommandWithRem
} }
private boolean tldContainsDomains(String tld) { private boolean tldContainsDomains(String tld) {
if (tm().isOfy()) { return transactIfJpaTm(
return ofy().load().type(DomainBase.class).filter("tld", tld).limit(1).count() > 0;
} else {
return jpaTm()
.transact(
() -> () ->
jpaTm() tm().createQueryComposer(DomainBase.class)
.query("FROM Domain WHERE tld = :tld", DomainBase.class) .where("tld", Comparator.EQ, tld)
.setParameter("tld", tld) .first()
.setMaxResults(1)
.getResultStream()
.findFirst()
.isPresent()); .isPresent());
} }
} }
}

View file

@ -16,9 +16,7 @@ package google.registry.tools;
import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.io.BaseEncoding.base16; import static com.google.common.io.BaseEncoding.base16;
import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.model.registry.Registries.assertTldExists; import static google.registry.model.registry.Registries.assertTldExists;
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm; import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm;
import static google.registry.util.DateTimeUtils.isBeforeOrAt; import static google.registry.util.DateTimeUtils.isBeforeOrAt;
@ -30,12 +28,14 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap;
import google.registry.model.domain.DomainBase; import google.registry.model.domain.DomainBase;
import google.registry.model.host.HostResource; import google.registry.model.host.HostResource;
import google.registry.persistence.transaction.QueryComposer.Comparator;
import google.registry.tools.params.PathParameter; import google.registry.tools.params.PathParameter;
import google.registry.util.Clock; import google.registry.util.Clock;
import java.net.InetAddress; import java.net.InetAddress;
import java.nio.file.Files; import java.nio.file.Files;
import java.nio.file.Path; import java.nio.file.Path;
import java.nio.file.Paths; import java.nio.file.Paths;
import java.util.List;
import java.util.Map; import java.util.Map;
import javax.inject.Inject; import javax.inject.Inject;
import org.joda.time.DateTime; import org.joda.time.DateTime;
@ -74,15 +74,12 @@ final class GenerateDnsReportCommand implements CommandWithRemoteApi {
String generate() { String generate() {
result.append("[\n"); result.append("[\n");
Iterable<DomainBase> domains = List<DomainBase> domains =
tm().isOfy() transactIfJpaTm(
? ofy().load().type(DomainBase.class).filter("tld", tld)
: tm().transact(
() -> () ->
jpaTm() tm().createQueryComposer(DomainBase.class)
.query("FROM Domain WHERE tld = :tld", DomainBase.class) .where("tld", Comparator.EQ, tld)
.setParameter("tld", tld) .list());
.getResultList());
for (DomainBase domain : domains) { for (DomainBase domain : domains) {
// Skip deleted domains and domains that don't get published to DNS. // Skip deleted domains and domains that don't get published to DNS.
if (isBeforeOrAt(domain.getDeletionTime(), now) || !domain.shouldPublishToDns()) { if (isBeforeOrAt(domain.getDeletionTime(), now) || !domain.shouldPublishToDns()) {

View file

@ -15,7 +15,7 @@
package google.registry.tools; package google.registry.tools;
import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkNotNull;
import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.ObjectifyService.auditedOfy;
import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameter;
import com.beust.jcommander.Parameters; import com.beust.jcommander.Parameters;
@ -47,7 +47,7 @@ final class GetResourceByKeyCommand implements CommandWithRemoteApi {
checkNotNull(Key.create(keyString), "Could not parse key string: " + keyString); checkNotNull(Key.create(keyString), "Could not parse key string: " + keyString);
EppResource resource = EppResource resource =
checkNotNull( checkNotNull(
ofy().load().key(resourceKey).now(), auditedOfy().load().key(resourceKey).now(),
"Could not load resource for key: " + resourceKey); "Could not load resource for key: " + resourceKey);
System.out.println(expand ? resource.toHydratedString() : resource.toString()); System.out.println(expand ? resource.toHydratedString() : resource.toString());
} }

View file

@ -15,7 +15,7 @@
package google.registry.tools; package google.registry.tools;
import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Preconditions.checkState;
import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.ObjectifyService.auditedOfy;
import static java.nio.charset.StandardCharsets.UTF_8; import static java.nio.charset.StandardCharsets.UTF_8;
import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameter;
@ -64,7 +64,7 @@ abstract class ReadEntityFromKeyPathCommand<T> extends MutatingCommand {
: Files.readLines(keyPathsFile, UTF_8); : Files.readLines(keyPathsFile, UTF_8);
for (String keyPath : keyPaths) { for (String keyPath : keyPaths) {
Key<?> untypedKey = parseKeyPath(keyPath); Key<?> untypedKey = parseKeyPath(keyPath);
Object entity = ofy().load().key(untypedKey).now(); Object entity = auditedOfy().load().key(untypedKey).now();
if (entity == null) { if (entity == null) {
System.err.printf( System.err.printf(
"Entity %s read from %s doesn't exist in Datastore! Skipping.%n", "Entity %s read from %s doesn't exist in Datastore! Skipping.%n",

View file

@ -15,7 +15,7 @@
package google.registry.tools; package google.registry.tools;
import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Preconditions.checkState;
import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.tools.Injector.injectReflectively; import static google.registry.tools.Injector.injectReflectively;
import static java.nio.charset.StandardCharsets.UTF_8; import static java.nio.charset.StandardCharsets.UTF_8;
@ -244,7 +244,7 @@ final class RegistryCli implements AutoCloseable, CommandRunner {
ObjectifyService.initOfy(); ObjectifyService.initOfy();
// Make sure we start the command with a clean cache, so that any previous command won't // Make sure we start the command with a clean cache, so that any previous command won't
// interfere with this one. // interfere with this one.
ofy().clearSessionCache(); tm().clearSessionCache();
// Enable Cloud SQL for command that needs remote API as they will very likely use // Enable Cloud SQL for command that needs remote API as they will very likely use
// Cloud SQL after the database migration. Note that the DB password is stored in Datastore // Cloud SQL after the database migration. Note that the DB password is stored in Datastore

View file

@ -15,7 +15,7 @@
package google.registry.tools; package google.registry.tools;
import static com.google.common.collect.Lists.partition; import static com.google.common.collect.Lists.partition;
import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.ObjectifyService.auditedOfy;
import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameter;
import com.beust.jcommander.Parameters; import com.beust.jcommander.Parameters;
@ -44,7 +44,8 @@ public final class ResaveEntitiesCommand extends MutatingCommand {
protected void init() { protected void init() {
for (List<String> batch : partition(mainParameters, BATCH_SIZE)) { for (List<String> batch : partition(mainParameters, BATCH_SIZE)) {
for (String websafeKey : batch) { for (String websafeKey : batch) {
ImmutableObject entity = ofy().load().key(Key.<ImmutableObject>create(websafeKey)).now(); ImmutableObject entity =
auditedOfy().load().key(Key.<ImmutableObject>create(websafeKey)).now();
stageEntityChange(entity, entity); stageEntityChange(entity, entity);
} }
flushTransaction(); flushTransaction();

View file

@ -16,7 +16,7 @@ package google.registry.tools;
import static com.google.common.collect.Lists.partition; import static com.google.common.collect.Lists.partition;
import static google.registry.model.common.EntityGroupRoot.getCrossTldKey; import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.ObjectifyService.auditedOfy;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import com.beust.jcommander.Parameters; import com.beust.jcommander.Parameters;
@ -46,8 +46,9 @@ final class ResaveEnvironmentEntitiesCommand implements CommandWithRemoteApi {
private static <T> void batchSave(Class<T> clazz) { private static <T> void batchSave(Class<T> clazz) {
System.out.printf("Re-saving %s entities.\n", clazz.getSimpleName()); System.out.printf("Re-saving %s entities.\n", clazz.getSimpleName());
for (final Iterable<Key<T>> batch : for (final Iterable<Key<T>> batch :
partition(ofy().load().type(clazz).ancestor(getCrossTldKey()).keys().list(), BATCH_SIZE)) { partition(
tm().transact(() -> ofy().save().entities(ofy().load().keys(batch).values())); auditedOfy().load().type(clazz).ancestor(getCrossTldKey()).keys().list(), BATCH_SIZE)) {
tm().transact(() -> auditedOfy().save().entities(auditedOfy().load().keys(batch).values()));
System.out.printf("Re-saved entities batch: %s.\n", batch); System.out.printf("Re-saved entities batch: %s.\n", batch);
} }
} }

View file

@ -17,9 +17,9 @@ package google.registry.tools.javascrap;
import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableListMultimap.flatteningToImmutableListMultimap; import static com.google.common.collect.ImmutableListMultimap.flatteningToImmutableListMultimap;
import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm;
import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameter;
import com.beust.jcommander.Parameters; import com.beust.jcommander.Parameters;
@ -31,6 +31,7 @@ import google.registry.model.domain.DomainBase;
import google.registry.model.reporting.Spec11ThreatMatch; import google.registry.model.reporting.Spec11ThreatMatch;
import google.registry.model.reporting.Spec11ThreatMatch.ThreatType; import google.registry.model.reporting.Spec11ThreatMatch.ThreatType;
import google.registry.model.reporting.Spec11ThreatMatchDao; import google.registry.model.reporting.Spec11ThreatMatchDao;
import google.registry.persistence.transaction.QueryComposer;
import google.registry.reporting.spec11.RegistrarThreatMatches; import google.registry.reporting.spec11.RegistrarThreatMatches;
import google.registry.reporting.spec11.Spec11RegistrarThreatMatchesParser; import google.registry.reporting.spec11.Spec11RegistrarThreatMatchesParser;
import google.registry.tools.CommandWithRemoteApi; import google.registry.tools.CommandWithRemoteApi;
@ -148,21 +149,14 @@ public class BackfillSpec11ThreatMatchesCommand extends ConfirmingCommand
/** Loads in all {@link DomainBase} objects for a given FQDN. */ /** Loads in all {@link DomainBase} objects for a given FQDN. */
private List<DomainBase> loadDomainsForFqdn(String fullyQualifiedDomainName) { private List<DomainBase> loadDomainsForFqdn(String fullyQualifiedDomainName) {
if (tm().isOfy()) { return transactIfJpaTm(
return ofy()
.load()
.type(DomainBase.class)
.filter("fullyQualifiedDomainName", fullyQualifiedDomainName)
.list();
} else {
return jpaTm()
.transact(
() -> () ->
jpaTm() tm().createQueryComposer(DomainBase.class)
.query("FROM Domain WHERE fullyQualifiedDomainName = :fqdn", DomainBase.class) .where(
.setParameter("fqdn", fullyQualifiedDomainName) "fullyQualifiedDomainName",
.getResultList()); QueryComposer.Comparator.EQ,
} fullyQualifiedDomainName)
.list());
} }
/** Converts the previous {@link ThreatMatch} object to {@link Spec11ThreatMatch}. */ /** Converts the previous {@link ThreatMatch} object to {@link Spec11ThreatMatch}. */

View file

@ -15,7 +15,7 @@
package google.registry.tools.javascrap; package google.registry.tools.javascrap;
import static com.google.common.base.Verify.verify; import static com.google.common.base.Verify.verify;
import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.ObjectifyService.auditedOfy;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameter;
@ -52,12 +52,12 @@ public class DeleteContactByRoidCommand extends ConfirmingCommand implements Com
ImmutableList<Key<?>> toDelete; ImmutableList<Key<?>> toDelete;
@Override @Override
protected void init() throws Exception { protected void init() {
System.out.printf("Deleting %s, which refers to %s.\n", roid, contactId); System.out.printf("Deleting %s, which refers to %s.\n", roid, contactId);
tm().transact( tm().transact(
() -> { () -> {
Key<ContactResource> targetKey = Key.create(ContactResource.class, roid); Key<ContactResource> targetKey = Key.create(ContactResource.class, roid);
ContactResource targetContact = ofy().load().key(targetKey).now(); ContactResource targetContact = auditedOfy().load().key(targetKey).now();
verify( verify(
Objects.equals(targetContact.getContactId(), contactId), Objects.equals(targetContact.getContactId(), contactId),
"contactId does not match."); "contactId does not match.");
@ -76,7 +76,7 @@ public class DeleteContactByRoidCommand extends ConfirmingCommand implements Com
roid, canonicalResource); roid, canonicalResource);
List<Object> ancestors = List<Object> ancestors =
ofy().load().ancestor(Key.create(ContactResource.class, roid)).list(); auditedOfy().load().ancestor(Key.create(ContactResource.class, roid)).list();
System.out.println("Ancestor query returns: "); System.out.println("Ancestor query returns: ");
for (Object entity : ancestors) { for (Object entity : ancestors) {
@ -92,7 +92,7 @@ public class DeleteContactByRoidCommand extends ConfirmingCommand implements Com
.collect(ImmutableList.toImmutableList()); .collect(ImmutableList.toImmutableList());
EppResourceIndex eppResourceIndex = EppResourceIndex eppResourceIndex =
ofy().load().entity(EppResourceIndex.create(targetKey)).now(); auditedOfy().load().entity(EppResourceIndex.create(targetKey)).now();
verify(eppResourceIndex.getKey().equals(targetKey), "Wrong EppResource Index loaded"); verify(eppResourceIndex.getKey().equals(targetKey), "Wrong EppResource Index loaded");
System.out.printf("\n\nEppResourceIndex found (%s).\n", Key.create(eppResourceIndex)); System.out.printf("\n\nEppResourceIndex found (%s).\n", Key.create(eppResourceIndex));
@ -103,13 +103,13 @@ public class DeleteContactByRoidCommand extends ConfirmingCommand implements Com
.build(); .build();
System.out.printf("\n\nAbout to delete %s entities:\n", toDelete.size()); System.out.printf("\n\nAbout to delete %s entities:\n", toDelete.size());
toDelete.forEach(key -> System.out.println(key)); toDelete.forEach(System.out::println);
}); });
} }
@Override @Override
protected String execute() { protected String execute() {
tm().transact(() -> ofy().delete().keys(toDelete).now()); tm().transact(() -> auditedOfy().delete().keys(toDelete).now());
return "Done"; return "Done";
} }
} }

View file

@ -16,7 +16,7 @@ package google.registry.tools.server;
import static com.google.appengine.api.datastore.DatastoreServiceFactory.getDatastoreService; import static com.google.appengine.api.datastore.DatastoreServiceFactory.getDatastoreService;
import static com.googlecode.objectify.Key.create; import static com.googlecode.objectify.Key.create;
import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.ObjectifyService.auditedOfy;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import com.google.appengine.api.datastore.Entity; import com.google.appengine.api.datastore.Entity;
@ -95,7 +95,7 @@ public class DeleteEntityAction implements Runnable {
getDatastoreService().delete(rawDeletions); getDatastoreService().delete(rawDeletions);
// Delete ofy entities. // Delete ofy entities.
final ImmutableList<Object> ofyDeletions = ofyDeletionsBuilder.build(); final ImmutableList<Object> ofyDeletions = ofyDeletionsBuilder.build();
tm().transactNew(() -> ofy().delete().entities(ofyDeletions).now()); tm().transactNew(() -> auditedOfy().delete().entities(ofyDeletions).now());
String message = String.format( String message = String.format(
"Deleted %d raw entities and %d registered entities", "Deleted %d raw entities and %d registered entities",
rawDeletions.size(), rawDeletions.size(),
@ -106,8 +106,9 @@ public class DeleteEntityAction implements Runnable {
private Optional<Object> loadOfyEntity(Key rawKey) { private Optional<Object> loadOfyEntity(Key rawKey) {
try { try {
EntityMetadata<Object> metadata = ofy().factory().getMetadata(rawKey.getKind()); EntityMetadata<Object> metadata = auditedOfy().factory().getMetadata(rawKey.getKind());
return Optional.ofNullable(metadata == null ? null : ofy().load().key(create(rawKey)).now()); return Optional.ofNullable(
metadata == null ? null : auditedOfy().load().key(create(rawKey)).now());
} catch (Throwable e) { } catch (Throwable e) {
logger.atWarning().withCause(e).log( logger.atWarning().withCause(e).log(
"Could not load entity with key %s using Objectify; falling back to raw Datastore.", "Could not load entity with key %s using Objectify; falling back to raw Datastore.",

View file

@ -15,7 +15,7 @@
package google.registry.tools.server; package google.registry.tools.server;
import static com.google.common.collect.Iterators.partition; import static com.google.common.collect.Iterators.partition;
import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.ObjectifyService.auditedOfy;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import com.google.appengine.tools.mapreduce.Reducer; import com.google.appengine.tools.mapreduce.Reducer;
@ -37,7 +37,7 @@ public class KillAllEntitiesReducer extends Reducer<Key<?>, Key<?>, Void> {
while (batches.hasNext()) { while (batches.hasNext()) {
final List<Key<?>> batch = batches.next(); final List<Key<?>> batch = batches.next();
// Use a transaction to get retrying for free. // Use a transaction to get retrying for free.
tm().transact(() -> ofy().deleteWithoutBackup().keys(batch)); tm().transact(() -> auditedOfy().deleteWithoutBackup().keys(batch));
getContext().incrementCounter("entities deleted", batch.size()); getContext().incrementCounter("entities deleted", batch.size());
for (Key<?> key : batch) { for (Key<?> key : batch) {
getContext().incrementCounter(String.format("%s deleted", key.getKind())); getContext().incrementCounter(String.format("%s deleted", key.getKind()));

View file

@ -15,7 +15,7 @@
package google.registry.tools.server; package google.registry.tools.server;
import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkArgument;
import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.ObjectifyService.auditedOfy;
import static google.registry.request.Action.Method.POST; import static google.registry.request.Action.Method.POST;
import com.google.appengine.tools.mapreduce.Mapper; import com.google.appengine.tools.mapreduce.Mapper;
@ -90,10 +90,10 @@ public class KillAllEppResourcesAction implements Runnable {
Key<EppResourceIndex> eriKey = Key.create(eri); Key<EppResourceIndex> eriKey = Key.create(eri);
emitAndIncrementCounter(eriKey, eriKey); emitAndIncrementCounter(eriKey, eriKey);
Key<?> resourceKey = eri.getKey(); Key<?> resourceKey = eri.getKey();
for (Key<Object> key : ofy().load().ancestor(resourceKey).keys()) { for (Key<Object> key : auditedOfy().load().ancestor(resourceKey).keys()) {
emitAndIncrementCounter(resourceKey, key); emitAndIncrementCounter(resourceKey, key);
} }
EppResource resource = ofy().load().key(eri.getKey()).now(); EppResource resource = auditedOfy().load().key(eri.getKey()).now();
// TODO(b/28247733): What about FKI's for renamed hosts? // TODO(b/28247733): What about FKI's for renamed hosts?
Key<?> indexKey = ForeignKeyIndex.createKey(resource); Key<?> indexKey = ForeignKeyIndex.createKey(resource);
emitAndIncrementCounter(indexKey, indexKey); emitAndIncrementCounter(indexKey, indexKey);

View file

@ -16,7 +16,7 @@ package google.registry.tools.server;
import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableList.toImmutableList;
import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.ObjectifyService.auditedOfy;
import static google.registry.model.registry.Registries.assertTldsExist; import static google.registry.model.registry.Registries.assertTldsExist;
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
@ -86,7 +86,7 @@ public final class ListDomainsAction extends ListObjectsAction<DomainBase> {
// Combine the batches together by sorting all domains together with newest first, applying the // Combine the batches together by sorting all domains together with newest first, applying the
// limit, and then reversing for display order. // limit, and then reversing for display order.
for (List<String> tldsBatch : Lists.partition(tlds.asList(), maxNumSubqueries)) { for (List<String> tldsBatch : Lists.partition(tlds.asList(), maxNumSubqueries)) {
ofy() auditedOfy()
.load() .load()
.type(DomainBase.class) .type(DomainBase.class)
.filter("tld in", tldsBatch) .filter("tld in", tldsBatch)

View file

@ -14,7 +14,7 @@
package google.registry.tools.server; package google.registry.tools.server;
import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.ObjectifyService.auditedOfy;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import com.google.appengine.tools.mapreduce.Mapper; import com.google.appengine.tools.mapreduce.Mapper;
@ -69,7 +69,9 @@ public class ResaveAllHistoryEntriesAction implements Runnable {
@Override @Override
public final void map(final HistoryEntry historyEntry) { public final void map(final HistoryEntry historyEntry) {
tm().transact(() -> ofy().save().entity(ofy().load().entity(historyEntry).now()).now()); tm().transact(
() ->
auditedOfy().save().entity(auditedOfy().load().entity(historyEntry).now()).now());
getContext().incrementCounter( getContext().incrementCounter(
String.format( String.format(
"HistoryEntries parented under %s re-saved", historyEntry.getParent().getKind())); "HistoryEntries parented under %s re-saved", historyEntry.getParent().getKind()));

View file

@ -30,6 +30,8 @@ import google.registry.testing.AppEngineExtension;
import google.registry.testing.DualDatabaseTest; import google.registry.testing.DualDatabaseTest;
import google.registry.testing.FakeClock; import google.registry.testing.FakeClock;
import google.registry.testing.TestOfyAndSql; import google.registry.testing.TestOfyAndSql;
import google.registry.testing.TestOfyOnly;
import google.registry.testing.TestSqlOnly;
import java.util.Optional; import java.util.Optional;
import javax.persistence.Column; import javax.persistence.Column;
import javax.persistence.NoResultException; import javax.persistence.NoResultException;
@ -263,6 +265,33 @@ public class QueryComposerTest {
.isEqualTo(ImmutableList.of()); .isEqualTo(ImmutableList.of());
} }
@TestOfyOnly
void testMultipleInequalities_failsDatastore() {
assertThat(
assertThrows(
IllegalArgumentException.class,
() ->
tm().createQueryComposer(TestEntity.class)
.where("val", Comparator.GT, 1)
.where("name", Comparator.LT, "b")
.list()))
.hasMessageThat()
.isEqualTo(
"Datastore cannot handle inequality queries on multiple fields, we found 2 fields.");
}
@TestSqlOnly
void testMultipleInequalities_succeedsSql() {
assertThat(
transactIfJpaTm(
() ->
tm().createQueryComposer(TestEntity.class)
.where("val", Comparator.GT, 1)
.where("name", Comparator.LT, "b")
.list()))
.containsExactly(alpha);
}
@javax.persistence.Entity @javax.persistence.Entity
@Entity(name = "QueryComposerTestEntity") @Entity(name = "QueryComposerTestEntity")
private static class TestEntity extends ImmutableObject { private static class TestEntity extends ImmutableObject {