Remove ofy support from ServerSecret (#1773)

This commit is contained in:
Lai Jiang 2022-09-09 10:38:12 -04:00 committed by GitHub
parent ac1ffacf7d
commit adb9162a98
5 changed files with 10 additions and 55 deletions

View file

@ -26,7 +26,6 @@ import google.registry.model.host.HostHistory;
import google.registry.model.index.EppResourceIndex; import google.registry.model.index.EppResourceIndex;
import google.registry.model.index.EppResourceIndexBucket; import google.registry.model.index.EppResourceIndexBucket;
import google.registry.model.reporting.HistoryEntry; import google.registry.model.reporting.HistoryEntry;
import google.registry.model.server.ServerSecret;
/** Sets of classes of the Objectify-registered entities in use throughout the model. */ /** Sets of classes of the Objectify-registered entities in use throughout the model. */
@DeleteAfterMigration @DeleteAfterMigration
@ -44,8 +43,7 @@ public final class EntityClasses {
GaeUserIdConverter.class, GaeUserIdConverter.class,
HistoryEntry.class, HistoryEntry.class,
Host.class, Host.class,
HostHistory.class, HostHistory.class);
ServerSecret.class);
private EntityClasses() {} private EntityClasses() {}
} }

View file

@ -19,7 +19,12 @@ import google.registry.model.ImmutableObject;
import google.registry.model.annotations.DeleteAfterMigration; import google.registry.model.annotations.DeleteAfterMigration;
import javax.persistence.MappedSuperclass; import javax.persistence.MappedSuperclass;
/** A singleton entity in the database. */ /**
* A singleton entity in the database.
*
* <p>This class should not be deleted after the migration, because there is still a concept of
* singleton in SQL. We should remove the ofy @Id annotation after all of its subclass are Ofy-free.
*/
@DeleteAfterMigration @DeleteAfterMigration
@MappedSuperclass @MappedSuperclass
public abstract class CrossTldSingleton extends ImmutableObject { public abstract class CrossTldSingleton extends ImmutableObject {

View file

@ -19,27 +19,16 @@ import static google.registry.persistence.transaction.TransactionManagerFactory.
import com.github.benmanes.caffeine.cache.LoadingCache; import com.github.benmanes.caffeine.cache.LoadingCache;
import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import com.google.common.primitives.Longs; import com.google.common.primitives.Longs;
import com.googlecode.objectify.annotation.Entity;
import com.googlecode.objectify.annotation.Ignore;
import com.googlecode.objectify.annotation.OnLoad;
import com.googlecode.objectify.annotation.Unindex;
import google.registry.model.CacheUtils; import google.registry.model.CacheUtils;
import google.registry.model.annotations.NotBackedUp;
import google.registry.model.annotations.NotBackedUp.Reason;
import google.registry.model.common.CrossTldSingleton; import google.registry.model.common.CrossTldSingleton;
import java.nio.ByteBuffer; import java.nio.ByteBuffer;
import java.util.Optional; import java.util.Optional;
import java.util.UUID; import java.util.UUID;
import javax.persistence.Column; import javax.persistence.Column;
import javax.persistence.PostLoad; import javax.persistence.Entity;
import javax.persistence.Transient;
/** A secret number used for generating tokens (such as XSRF tokens). */ /** A secret number used for generating tokens (such as XSRF tokens). */
@Entity @Entity
@javax.persistence.Entity
@Unindex
@NotBackedUp(reason = Reason.AUTO_GENERATED)
// TODO(b/27427316): Replace this with an entry in KMSKeyring
public class ServerSecret extends CrossTldSingleton { public class ServerSecret extends CrossTldSingleton {
/** /**
@ -52,13 +41,6 @@ public class ServerSecret extends CrossTldSingleton {
CacheUtils.newCacheBuilder().build(singletonClazz -> retrieveAndSaveSecret()); CacheUtils.newCacheBuilder().build(singletonClazz -> retrieveAndSaveSecret());
private static ServerSecret retrieveAndSaveSecret() { private static ServerSecret retrieveAndSaveSecret() {
if (tm().isOfy()) {
// Attempt a quick load if we're in ofy first to short-circuit sans transaction
Optional<ServerSecret> secretWithoutTransaction = tm().loadSingleton(ServerSecret.class);
if (secretWithoutTransaction.isPresent()) {
return secretWithoutTransaction.get();
}
}
return tm().transact( return tm().transact(
() -> { () -> {
// Make sure we're in a transaction and attempt to load any existing secret, then // Make sure we're in a transaction and attempt to load any existing secret, then
@ -77,35 +59,13 @@ public class ServerSecret extends CrossTldSingleton {
return CACHE.get(ServerSecret.class); return CACHE.get(ServerSecret.class);
} }
/** Most significant 8 bytes of the UUID value (stored separately for legacy purposes). */
@Transient long mostSignificant;
/** Least significant 8 bytes of the UUID value (stored separately for legacy purposes). */
@Transient long leastSignificant;
/** The UUID value itself. */ /** The UUID value itself. */
@Column(columnDefinition = "uuid") @Column(columnDefinition = "uuid")
@Ignore
UUID secret; UUID secret;
/** Convert the Datastore representation to SQL. */
@OnLoad
void onLoad() {
secret = new UUID(mostSignificant, leastSignificant);
}
/** Convert the SQL representation to Datastore. */
@PostLoad
void postLoad() {
mostSignificant = secret.getMostSignificantBits();
leastSignificant = secret.getLeastSignificantBits();
}
@VisibleForTesting @VisibleForTesting
static ServerSecret create(UUID uuid) { static ServerSecret create(UUID uuid) {
ServerSecret secret = new ServerSecret(); ServerSecret secret = new ServerSecret();
secret.mostSignificant = uuid.getMostSignificantBits();
secret.leastSignificant = uuid.getLeastSignificantBits();
secret.secret = uuid; secret.secret = uuid;
return secret; return secret;
} }
@ -113,8 +73,8 @@ public class ServerSecret extends CrossTldSingleton {
/** Returns the value of this ServerSecret as a byte array. */ /** Returns the value of this ServerSecret as a byte array. */
public byte[] asBytes() { public byte[] asBytes() {
return ByteBuffer.allocate(Longs.BYTES * 2) return ByteBuffer.allocate(Longs.BYTES * 2)
.putLong(mostSignificant) .putLong(secret.getMostSignificantBits())
.putLong(leastSignificant) .putLong(secret.getLeastSignificantBits())
.array(); .array();
} }

View file

@ -24,7 +24,6 @@ import google.registry.model.host.Host;
import google.registry.model.index.EppResourceIndex; import google.registry.model.index.EppResourceIndex;
import google.registry.model.index.EppResourceIndexBucket; import google.registry.model.index.EppResourceIndexBucket;
import google.registry.model.reporting.HistoryEntry; import google.registry.model.reporting.HistoryEntry;
import google.registry.model.server.ServerSecret;
import google.registry.testing.TestObject; import google.registry.testing.TestObject;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
@ -48,7 +47,6 @@ public class ClassPathManagerTest {
.isEqualTo(EppResourceIndexBucket.class); .isEqualTo(EppResourceIndexBucket.class);
assertThat(ClassPathManager.getClass("Domain")).isEqualTo(Domain.class); assertThat(ClassPathManager.getClass("Domain")).isEqualTo(Domain.class);
assertThat(ClassPathManager.getClass("HistoryEntry")).isEqualTo(HistoryEntry.class); assertThat(ClassPathManager.getClass("HistoryEntry")).isEqualTo(HistoryEntry.class);
assertThat(ClassPathManager.getClass("ServerSecret")).isEqualTo(ServerSecret.class);
assertThat(ClassPathManager.getClass("EppResourceIndex")).isEqualTo(EppResourceIndex.class); assertThat(ClassPathManager.getClass("EppResourceIndex")).isEqualTo(EppResourceIndex.class);
} }
@ -90,7 +88,6 @@ public class ClassPathManagerTest {
.isEqualTo("EppResourceIndexBucket"); .isEqualTo("EppResourceIndexBucket");
assertThat(ClassPathManager.getClassName(Domain.class)).isEqualTo("Domain"); assertThat(ClassPathManager.getClassName(Domain.class)).isEqualTo("Domain");
assertThat(ClassPathManager.getClassName(HistoryEntry.class)).isEqualTo("HistoryEntry"); assertThat(ClassPathManager.getClassName(HistoryEntry.class)).isEqualTo("HistoryEntry");
assertThat(ClassPathManager.getClassName(ServerSecret.class)).isEqualTo("ServerSecret");
assertThat(ClassPathManager.getClassName(EppResourceIndex.class)).isEqualTo("EppResourceIndex"); assertThat(ClassPathManager.getClassName(EppResourceIndex.class)).isEqualTo("EppResourceIndex");
} }

View file

@ -188,8 +188,3 @@ enum google.registry.model.reporting.HistoryEntry$Type {
RDE_IMPORT; RDE_IMPORT;
SYNTHETIC; SYNTHETIC;
} }
class google.registry.model.server.ServerSecret {
@Id long id;
long leastSignificant;
long mostSignificant;
}