Replace some unnecessary uses of "real" ofy() with Ofy.

Only Ofy itself and its two helpers (AugmentedSaver and
AugmentedDeleter) need to use the real ofy(). All other
callers should be using Ofy. Fixing this even though it
doesn't change anything because I found it baffling to
follow the code while trying to make a small change.

Update: added a presubmit to enforce this.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=154456603
This commit is contained in:
cgoldfeder 2017-04-27 12:40:36 -07:00 committed by Ben McIlwain
parent 927eb43cbc
commit 0267214841
5 changed files with 25 additions and 21 deletions

View file

@ -16,8 +16,8 @@ package google.registry.model.ofy;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.DiscreteDomain.integers;
import static com.googlecode.objectify.ObjectifyService.ofy;
import static google.registry.config.RegistryConfig.getCommitLogBucketCount;
import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.util.DateTimeUtils.START_OF_TIME;
import com.google.common.base.Function;

View file

@ -14,7 +14,7 @@
package google.registry.model.ofy;
import static com.googlecode.objectify.ObjectifyService.ofy;
import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.util.DateTimeUtils.START_OF_TIME;
import com.googlecode.objectify.Key;

View file

@ -20,8 +20,8 @@ import static com.google.common.base.Predicates.not;
import static com.google.common.collect.Maps.filterKeys;
import static com.google.common.collect.Sets.difference;
import static com.google.common.collect.Sets.union;
import static com.googlecode.objectify.ObjectifyService.ofy;
import static google.registry.model.ofy.CommitLogBucket.loadBucket;
import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.util.DateTimeUtils.isBeforeOrAt;
import com.google.common.base.Function;
@ -132,10 +132,15 @@ class CommitLoggedWork<R> extends VoidWork {
if (isBeforeOrAt(info.transactionTime, bucket.getLastWrittenTime())) {
throw new TimestampInversionException(info.transactionTime, bucket.getLastWrittenTime());
}
// The keys read by Objectify during this transaction. This won't include the keys of
// asynchronous save and delete operations that haven't been reaped, but that's ok because we
// already logged all of those keys in {@link TransactionInfo} and now just need to figure out
// what was loaded.
ImmutableSet<Key<?>> keysInSessionCache = ofy().getSessionKeys();
Map<Key<BackupGroupRoot>, BackupGroupRoot> rootsForTouchedKeys =
getBackupGroupRoots(touchedKeys);
Map<Key<BackupGroupRoot>, BackupGroupRoot> rootsForUntouchedKeys =
getBackupGroupRoots(difference(getObjectifySessionCacheKeys(), touchedKeys));
getBackupGroupRoots(difference(keysInSessionCache, touchedKeys));
// Check the update timestamps of all keys in the transaction, whether touched or merely read.
checkBackupGroupRootTimestamps(
info.transactionTime,
@ -153,7 +158,7 @@ class CommitLoggedWork<R> extends VoidWork {
return CommitLogMutation.create(manifestKey, saveEntity);
}})
.toSet();
ofy().save()
ofy().saveWithoutBackup()
.entities(new ImmutableSet.Builder<>()
.add(manifest)
.add(bucket.asBuilder().setLastWrittenTime(info.transactionTime).build())
@ -163,17 +168,6 @@ class CommitLoggedWork<R> extends VoidWork {
.now();
}
/**
* Returns keys read by Objectify during this transaction.
*
* <p>This won't include the keys of asynchronous save and delete operations that haven't been
* reaped. But that's ok because we already logged all of those keys in {@link TransactionInfo}
* and only need this method to figure out what was loaded.
*/
private ImmutableSet<Key<?>> getObjectifySessionCacheKeys() {
return ((SessionKeyExposingObjectify) ofy()).getSessionKeys();
}
/** Check that the timestamp of each BackupGroupRoot is in the past. */
private void checkBackupGroupRootTimestamps(
DateTime transactionTime, Set<Entry<Key<BackupGroupRoot>, BackupGroupRoot>> bgrEntries) {

View file

@ -107,6 +107,16 @@ public class Ofy {
return ofy().factory();
}
/**
* Returns keys read by Objectify during this transaction.
*
* <p>This won't include the keys of asynchronous save and delete operations that haven't been
* reaped.
*/
public ImmutableSet<Key<?>> getSessionKeys() {
return ((SessionKeyExposingObjectify) ofy()).getSessionKeys();
}
/** Clears the session cache. */
public void clearSessionCache() {
ofy().clear();
@ -120,9 +130,9 @@ public class Ofy {
checkState(inTransaction(), "Must be called in a transaction");
}
/**
/**
* Load from Datastore, bypassing memcache even when the results might be there.
*
*
* <p>In general, this is the correct method to use for loads. Loading from memcache can, in rare
* instances, produce a stale result (when a memcache write fails and the previous result is not
* cleared out) and so using memcache should be avoided unless the caller can tolerate staleness
@ -134,9 +144,9 @@ public class Ofy {
return ofy().cache(true).load();
}
/**
/**
* Load from Datastore, bypassing memcache even when the results might be there.
*
*
* <p>In general, prefer {@link #load} over this method. Loading from memcache can, in rare
* instances, produce a stale result (when a memcache write fails and the previous result is not
* cleared out) and so using memcache should be avoided unless the caller can tolerate staleness

View file

@ -18,8 +18,8 @@ import static com.google.common.base.Functions.constant;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.Maps.filterValues;
import static com.google.common.collect.Maps.toMap;
import static com.googlecode.objectify.ObjectifyService.ofy;
import static google.registry.model.ofy.CommitLogBucket.getArbitraryBucketId;
import static google.registry.model.ofy.ObjectifyService.ofy;
import com.google.common.base.Predicate;
import com.google.common.base.Predicates;