From 1c96cd64fe759ae91428d5e1f4b74df8d7c438d9 Mon Sep 17 00:00:00 2001 From: Michael Muller Date: Fri, 16 Apr 2021 12:21:03 -0400 Subject: [PATCH] Implement query abstraction (#1069) * Implement query abstraction Implement a query abstraction layer ("QueryComposer") that allows us to construct fluent-style queries that work across both Objectify and JPA. As a demonstration of the concept, convert Spec11EmailUtils and its test to use the new API. Limitations: - The primary limitations of this system are imposed by datastore, for example all queryable fields must be indexed, orderBy must coincide with the order of any inequality queries, inequality filters are limited to one property... - JPA queries are limited to a set of where clauses (all of which must match) and an "order by" clause. Joins, functions, complex where logic and multi-table queries are simply not allowed. - Descending sort order is currently unsupported (this is simple enough to add). --- .../ofy/DatastoreTransactionManager.java | 51 ++++ .../transaction/CriteriaQueryBuilder.java | 13 +- .../JpaTransactionManagerImpl.java | 47 +++ .../transaction/QueryComposer.java | 190 +++++++++++++ .../transaction/TransactionManager.java | 3 + .../reporting/spec11/Spec11EmailUtils.java | 29 +- .../transaction/QueryComposerTest.java | 269 ++++++++++++++++++ .../spec11/Spec11EmailUtilsTest.java | 26 +- 8 files changed, 600 insertions(+), 28 deletions(-) create mode 100644 core/src/main/java/google/registry/persistence/transaction/QueryComposer.java create mode 100644 core/src/test/java/google/registry/persistence/transaction/QueryComposerTest.java diff --git a/core/src/main/java/google/registry/model/ofy/DatastoreTransactionManager.java b/core/src/main/java/google/registry/model/ofy/DatastoreTransactionManager.java index 1d006d403..a06ce5561 100644 --- a/core/src/main/java/google/registry/model/ofy/DatastoreTransactionManager.java +++ b/core/src/main/java/google/registry/model/ofy/DatastoreTransactionManager.java @@ -37,12 +37,17 @@ import google.registry.model.domain.DomainHistory; import google.registry.model.host.HostHistory; import google.registry.model.reporting.HistoryEntry; import google.registry.persistence.VKey; +import google.registry.persistence.transaction.QueryComposer; import google.registry.persistence.transaction.TransactionManager; +import java.util.List; import java.util.NoSuchElementException; import java.util.Optional; import java.util.function.Supplier; +import java.util.stream.Stream; import java.util.stream.StreamSupport; import javax.annotation.Nullable; +import javax.persistence.NoResultException; +import javax.persistence.NonUniqueResultException; import org.joda.time.DateTime; /** Datastore implementation of {@link TransactionManager}. */ @@ -302,6 +307,11 @@ public class DatastoreTransactionManager implements TransactionManager { syncIfTransactionless(getOfy().deleteWithoutBackup().entity(entity)); } + @Override + public QueryComposer createQueryComposer(Class entity) { + return new DatastoreQueryComposerImpl(entity); + } + @Override public void clearSessionCache() { getOfy().clearSessionCache(); @@ -363,4 +373,45 @@ public class DatastoreTransactionManager implements TransactionManager { } return obj; } + + private static class DatastoreQueryComposerImpl extends QueryComposer { + DatastoreQueryComposerImpl(Class entityClass) { + super(entityClass); + } + + Query buildQuery() { + Query result = ofy().load().type(entityClass); + for (WhereClause pred : predicates) { + result = result.filter(pred.fieldName + pred.comparator.getDatastoreString(), pred.value); + } + + if (orderBy != null) { + result = result.order(orderBy); + } + + return result; + } + + @Override + public Optional first() { + return Optional.ofNullable(buildQuery().first().now()); + } + + @Override + public T getSingleResult() { + List results = buildQuery().limit(2).list(); + if (results.size() == 0) { + // The exception text here is the same as what we get for JPA queries. + throw new NoResultException("No entity found for query"); + } else if (results.size() > 1) { + throw new NonUniqueResultException("More than one result found for getSingleResult query"); + } + return results.get(0); + } + + @Override + public Stream stream() { + return Streams.stream(buildQuery()); + } + } } diff --git a/core/src/main/java/google/registry/persistence/transaction/CriteriaQueryBuilder.java b/core/src/main/java/google/registry/persistence/transaction/CriteriaQueryBuilder.java index ec8cb39ec..c4a206103 100644 --- a/core/src/main/java/google/registry/persistence/transaction/CriteriaQueryBuilder.java +++ b/core/src/main/java/google/registry/persistence/transaction/CriteriaQueryBuilder.java @@ -18,6 +18,7 @@ import static google.registry.persistence.transaction.TransactionManagerFactory. import com.google.common.collect.ImmutableList; import java.util.Collection; +import javax.persistence.EntityManager; import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.CriteriaQuery; import javax.persistence.criteria.Expression; @@ -35,7 +36,7 @@ import javax.persistence.criteria.Root; public class CriteriaQueryBuilder { /** Functional interface that defines the 'where' operator, e.g. {@link CriteriaBuilder#equal}. */ - public interface WhereClause { + public interface WhereOperator { Predicate predicate(Expression expression, U object); } @@ -50,7 +51,8 @@ public class CriteriaQueryBuilder { } /** Adds a WHERE clause to the query, given the specified operation, field, and value. */ - public CriteriaQueryBuilder where(String fieldName, WhereClause whereClause, V value) { + public CriteriaQueryBuilder where( + String fieldName, WhereOperator whereClause, V value) { Expression expression = root.get(fieldName); return where(whereClause.predicate(expression, value)); } @@ -94,7 +96,12 @@ public class CriteriaQueryBuilder { /** Creates a query builder that will SELECT from the given class. */ public static CriteriaQueryBuilder create(Class clazz) { - CriteriaQuery query = jpaTm().getEntityManager().getCriteriaBuilder().createQuery(clazz); + return create(jpaTm().getEntityManager(), clazz); + } + + /** Creates a query builder for the given entity manager. */ + public static CriteriaQueryBuilder create(EntityManager em, Class clazz) { + CriteriaQuery query = em.getCriteriaBuilder().createQuery(clazz); Root root = query.from(clazz); query = query.select(root); return new CriteriaQueryBuilder<>(query, root); diff --git a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java index 82bea3b11..7c14557fb 100644 --- a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java +++ b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java @@ -43,10 +43,12 @@ import google.registry.util.Clock; import google.registry.util.Retrier; import google.registry.util.SystemSleeper; import java.lang.reflect.Field; +import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Optional; import java.util.function.Supplier; +import java.util.stream.Stream; import java.util.stream.StreamSupport; import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; @@ -530,6 +532,11 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { delete(entity); } + @Override + public QueryComposer createQueryComposer(Class entity) { + return new JpaQueryComposerImpl(entity, getEntityManager()); + } + @Override public void clearSessionCache() { // This is an intended no-op method as there is no session cache in Postgresql. @@ -681,4 +688,44 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { } } } + + private static class JpaQueryComposerImpl extends QueryComposer { + + EntityManager em; + + JpaQueryComposerImpl(Class entityClass, EntityManager em) { + super(entityClass); + this.em = em; + } + + private TypedQuery buildQuery() { + CriteriaQueryBuilder queryBuilder = CriteriaQueryBuilder.create(em, entityClass); + + for (WhereClause pred : predicates) { + pred.addToCriteriaQueryBuilder(queryBuilder); + } + + if (orderBy != null) { + queryBuilder.orderByAsc(orderBy); + } + + return em.createQuery(queryBuilder.build()); + } + + @Override + public Optional first() { + List results = buildQuery().setMaxResults(1).getResultList(); + return results.size() > 0 ? Optional.of(results.get(0)) : Optional.empty(); + } + + @Override + public T getSingleResult() { + return buildQuery().getSingleResult(); + } + + @Override + public Stream stream() { + return buildQuery().getResultStream(); + } + } } diff --git a/core/src/main/java/google/registry/persistence/transaction/QueryComposer.java b/core/src/main/java/google/registry/persistence/transaction/QueryComposer.java new file mode 100644 index 000000000..2785bb2f1 --- /dev/null +++ b/core/src/main/java/google/registry/persistence/transaction/QueryComposer.java @@ -0,0 +1,190 @@ +// Copyright 2021 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.persistence.transaction; + +import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; + +import com.google.common.base.Function; +import google.registry.persistence.transaction.CriteriaQueryBuilder.WhereOperator; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.stream.Stream; +import javax.annotation.Nullable; +import javax.persistence.criteria.CriteriaBuilder; + +/** + * Creates queries that can be used both for objectify and JPA. + * + *

Example usage: + * + * 

+ *    tm().createQueryComposer(EntityType.class)
+ *        .where("fieldName", Comparator.EQ, "value")
+ *        .orderBy("fieldName")
+ *        .stream()
+ *
+ */ +public abstract class QueryComposer { + + // The class whose entities we're querying. Note that this limits us to single table queries in + // SQL. In datastore, there's really no other kind of query. + protected Class entityClass; + + // Field to order by, if any. Null if we don't care about order. + @Nullable protected String orderBy; + + protected List> predicates = new ArrayList>(); + + protected QueryComposer(Class entityClass) { + this.entityClass = entityClass; + } + + /** + * Introduce a "where" clause to the query. + * + *

Causes the query to return only results where the field and value have the relationship + * specified by the comparator. For example, "field EQ value", "field GT value" etc. + */ + public > QueryComposer where( + String fieldName, Comparator comparator, U value) { + predicates.add(new WhereClause(fieldName, comparator, value)); + return this; + } + + /** + * Order the query results by the value of the specified field. + * + *

TODO(mmuller): add the ability to do descending sort order. + */ + public QueryComposer orderBy(String fieldName) { + orderBy = fieldName; + return this; + } + + /** Returns the first result of the query or an empty optional if there is none. */ + public abstract Optional first(); + + /** + * Returns the one and only result of a query. + * + *

Throws a {@link javax.persistence.NonUniqueResultException} if there is more than one + * result, throws {@link javax.persistence.NoResultException} if no results are found. + */ + public abstract T getSingleResult(); + + /** Returns the results of the query as a stream. */ + public abstract Stream stream(); + + // We have to wrap the CriteriaQueryBuilder predicate factories in our own functions because at + // the point where we pass them to the Comparator constructor, the compiler can't determine which + // of the overloads to use since there is no "value" object for context. + + public static > WhereOperator equal( + CriteriaBuilder criteriaBuilder) { + return criteriaBuilder::equal; + } + + public static > WhereOperator lessThan( + CriteriaBuilder criteriaBuilder) { + return criteriaBuilder::lessThan; + } + + public static > WhereOperator lessThanOrEqualTo( + CriteriaBuilder criteriaBuilder) { + return criteriaBuilder::lessThanOrEqualTo; + } + + public static > WhereOperator greaterThanOrEqualTo( + CriteriaBuilder criteriaBuilder) { + return criteriaBuilder::greaterThanOrEqualTo; + } + + public static > WhereOperator greaterThan( + CriteriaBuilder criteriaBuilder) { + return criteriaBuilder::greaterThan; + } + + /** + * Enum used to specify comparison operations, e.g. {@code where("fieldName", Comparator.NE, + * "someval")'}. + * + *

These contain values that specify the comparison behavior for both objectify and criteria + * queries. For objectify, we provide a string to be appended to the field name in a {@code + * filter()} expression. For criteria queries we provide a function that knows how to obtain a + * {@link WhereOperator} from a {@link CriteriaBuilder}. + * + *

Note that the objectify strings for comparators other than equality are preceded by a space + * because {@code filter()} expects the fieldname to be separated from the operator by a space. + */ + public enum Comparator { + /** + * Return only records whose field is equal to the value. + * + *

Note that the datastore string for this is empty, which is consistent with the way {@code + * filter()} works (it uses an unadorned field name to check for equality). + */ + EQ("", QueryComposer::equal), + + /** Return only records whose field is less than the value. */ + LT(" <", QueryComposer::lessThan), + + /** Return only records whose field is less than or equal to the value. */ + LTE(" <=", QueryComposer::lessThanOrEqualTo), + + /** Return only records whose field is greater than or equal to the value. */ + GTE(" >=", QueryComposer::greaterThanOrEqualTo), + + /** Return only records whose field is greater than the value. */ + GT(" >", QueryComposer::greaterThan); + + private final String datastoreString; + + @SuppressWarnings("ImmutableEnumChecker") // Functions are immutable. + private final Function> operatorFactory; + + Comparator( + String datastoreString, Function> operatorFactory) { + this.datastoreString = datastoreString; + this.operatorFactory = operatorFactory; + } + + public String getDatastoreString() { + return datastoreString; + } + + public Function> getComparisonFactory() { + return operatorFactory; + } + }; + + protected static class WhereClause> { + public String fieldName; + public Comparator comparator; + public U value; + + WhereClause(String fieldName, Comparator comparator, U value) { + this.fieldName = fieldName; + this.comparator = comparator; + this.value = value; + } + + public void addToCriteriaQueryBuilder(CriteriaQueryBuilder queryBuilder) { + CriteriaBuilder criteriaBuilder = jpaTm().getEntityManager().getCriteriaBuilder(); + queryBuilder.where( + fieldName, comparator.getComparisonFactory().apply(criteriaBuilder), value); + } + } +} diff --git a/core/src/main/java/google/registry/persistence/transaction/TransactionManager.java b/core/src/main/java/google/registry/persistence/transaction/TransactionManager.java index 5711af47f..06e7823cc 100644 --- a/core/src/main/java/google/registry/persistence/transaction/TransactionManager.java +++ b/core/src/main/java/google/registry/persistence/transaction/TransactionManager.java @@ -273,6 +273,9 @@ public interface TransactionManager { */ void deleteWithoutBackup(Object entity); + /** Returns a QueryComposer which can be used to perform queries against the current database. */ + QueryComposer createQueryComposer(Class entity); + /** Clears the session cache if the underlying database is Datastore, otherwise it is a no-op. */ void clearSessionCache(); diff --git a/core/src/main/java/google/registry/reporting/spec11/Spec11EmailUtils.java b/core/src/main/java/google/registry/reporting/spec11/Spec11EmailUtils.java index 0cb7783c7..47ce0cfec 100644 --- a/core/src/main/java/google/registry/reporting/spec11/Spec11EmailUtils.java +++ b/core/src/main/java/google/registry/reporting/spec11/Spec11EmailUtils.java @@ -17,7 +17,9 @@ package google.registry.reporting.spec11; import static com.google.common.base.Throwables.getRootCause; import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.io.Resources.getResource; -import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.persistence.transaction.QueryComposer.Comparator; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; +import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -129,17 +131,20 @@ public class Spec11EmailUtils { private RegistrarThreatMatches filterOutNonPublishedMatches( RegistrarThreatMatches registrarThreatMatches) { ImmutableList filteredMatches = - registrarThreatMatches.threatMatches().stream() - .filter( - threatMatch -> - ofy() - .load() - .type(DomainBase.class) - .filter("fullyQualifiedDomainName", threatMatch.fullyQualifiedDomainName()) - .first() - .now() - .shouldPublishToDns()) - .collect(toImmutableList()); + transactIfJpaTm( + () -> { + return registrarThreatMatches.threatMatches().stream() + .filter( + threatMatch -> + tm().createQueryComposer(DomainBase.class) + .where( + "fullyQualifiedDomainName", + Comparator.EQ, + threatMatch.fullyQualifiedDomainName()) + .getSingleResult() + .shouldPublishToDns()) + .collect(toImmutableList()); + }); return RegistrarThreatMatches.create(registrarThreatMatches.clientId(), filteredMatches); } diff --git a/core/src/test/java/google/registry/persistence/transaction/QueryComposerTest.java b/core/src/test/java/google/registry/persistence/transaction/QueryComposerTest.java new file mode 100644 index 000000000..6c9ff31da --- /dev/null +++ b/core/src/test/java/google/registry/persistence/transaction/QueryComposerTest.java @@ -0,0 +1,269 @@ +// Copyright 2021 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.persistence.transaction; + +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.truth.Truth.assertThat; +import static google.registry.persistence.transaction.QueryComposer.Comparator; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; +import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm; +import static org.junit.Assert.assertThrows; + +import com.google.common.collect.ImmutableList; +import com.googlecode.objectify.annotation.Entity; +import com.googlecode.objectify.annotation.Id; +import com.googlecode.objectify.annotation.Index; +import google.registry.model.ImmutableObject; +import google.registry.testing.AppEngineExtension; +import google.registry.testing.DualDatabaseTest; +import google.registry.testing.FakeClock; +import google.registry.testing.TestOfyAndSql; +import java.util.Optional; +import javax.persistence.Column; +import javax.persistence.NoResultException; +import javax.persistence.NonUniqueResultException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.extension.RegisterExtension; + +@DualDatabaseTest +public class QueryComposerTest { + + private final FakeClock fakeClock = new FakeClock(); + + TestEntity alpha = new TestEntity("alpha", 3); + TestEntity bravo = new TestEntity("bravo", 2); + TestEntity charlie = new TestEntity("charlie", 1); + + @RegisterExtension + public final AppEngineExtension appEngine = + AppEngineExtension.builder() + .withClock(fakeClock) + .withDatastoreAndCloudSql() + .withOfyTestEntities(TestEntity.class) + .withJpaUnitTestEntities(TestEntity.class) + .build(); + + public QueryComposerTest() {} + + @BeforeEach + void setUp() { + tm().transact( + () -> { + tm().insert(alpha); + tm().insert(bravo); + tm().insert(charlie); + }); + } + + @TestOfyAndSql + public void testFirstQueries() { + assertThat( + transactIfJpaTm( + () -> + tm().createQueryComposer(TestEntity.class) + .where("name", Comparator.EQ, "bravo") + .first() + .get())) + .isEqualTo(bravo); + assertThat( + transactIfJpaTm( + () -> + tm().createQueryComposer(TestEntity.class) + .where("name", Comparator.GT, "bravo") + .first() + .get())) + .isEqualTo(charlie); + assertThat( + transactIfJpaTm( + () -> + tm().createQueryComposer(TestEntity.class) + .where("name", Comparator.GTE, "charlie") + .first() + .get())) + .isEqualTo(charlie); + assertThat( + transactIfJpaTm( + () -> + tm().createQueryComposer(TestEntity.class) + .where("name", Comparator.LT, "bravo") + .first() + .get())) + .isEqualTo(alpha); + assertThat( + transactIfJpaTm( + () -> + tm().createQueryComposer(TestEntity.class) + .where("name", Comparator.LTE, "alpha") + .first() + .get())) + .isEqualTo(alpha); + } + + @TestOfyAndSql + public void testGetSingleResult() { + assertThat( + transactIfJpaTm( + () -> + tm().createQueryComposer(TestEntity.class) + .where("name", Comparator.EQ, "alpha") + .getSingleResult())) + .isEqualTo(alpha); + } + + @TestOfyAndSql + public void testGetSingleResult_noResults() { + assertThrows( + NoResultException.class, + () -> + transactIfJpaTm( + () -> + tm().createQueryComposer(TestEntity.class) + .where("name", Comparator.EQ, "ziggy") + .getSingleResult())); + } + + @TestOfyAndSql + public void testGetSingleResult_nonUniqueResult() { + assertThrows( + NonUniqueResultException.class, + () -> + transactIfJpaTm( + () -> + tm().createQueryComposer(TestEntity.class) + .where("name", Comparator.GT, "alpha") + .getSingleResult())); + } + + @TestOfyAndSql + public void testStreamQueries() { + assertThat( + transactIfJpaTm( + () -> + tm() + .createQueryComposer(TestEntity.class) + .where("name", Comparator.EQ, "alpha") + .stream() + .collect(toImmutableList()))) + .isEqualTo(ImmutableList.of(alpha)); + assertThat( + transactIfJpaTm( + () -> + tm() + .createQueryComposer(TestEntity.class) + .where("name", Comparator.GT, "alpha") + .stream() + .collect(toImmutableList()))) + .isEqualTo(ImmutableList.of(bravo, charlie)); + assertThat( + transactIfJpaTm( + () -> + tm() + .createQueryComposer(TestEntity.class) + .where("name", Comparator.GTE, "bravo") + .stream() + .collect(toImmutableList()))) + .isEqualTo(ImmutableList.of(bravo, charlie)); + assertThat( + transactIfJpaTm( + () -> + tm() + .createQueryComposer(TestEntity.class) + .where("name", Comparator.LT, "charlie") + .stream() + .collect(toImmutableList()))) + .isEqualTo(ImmutableList.of(alpha, bravo)); + assertThat( + transactIfJpaTm( + () -> + tm() + .createQueryComposer(TestEntity.class) + .where("name", Comparator.LTE, "bravo") + .stream() + .collect(toImmutableList()))) + .isEqualTo(ImmutableList.of(alpha, bravo)); + } + + @TestOfyAndSql + public void testNonPrimaryKey() { + assertThat( + transactIfJpaTm( + () -> + tm().createQueryComposer(TestEntity.class) + .where("val", Comparator.EQ, 2) + .first() + .get())) + .isEqualTo(bravo); + } + + @TestOfyAndSql + public void testOrderBy() { + assertThat( + transactIfJpaTm( + () -> + tm() + .createQueryComposer(TestEntity.class) + .where("val", Comparator.GT, 1) + .orderBy("val") + .stream() + .collect(toImmutableList()))) + .isEqualTo(ImmutableList.of(bravo, alpha)); + } + + @TestOfyAndSql + public void testEmptyQueries() { + assertThat( + transactIfJpaTm( + () -> + tm().createQueryComposer(TestEntity.class) + .where("name", Comparator.GT, "foxtrot") + .first())) + .isEqualTo(Optional.empty()); + assertThat( + transactIfJpaTm( + () -> + tm() + .createQueryComposer(TestEntity.class) + .where("name", Comparator.GT, "foxtrot") + .stream() + .collect(toImmutableList()))) + .isEqualTo(ImmutableList.of()); + } + + @javax.persistence.Entity + @Entity(name = "QueryComposerTestEntity") + private static class TestEntity extends ImmutableObject { + @javax.persistence.Id @Id private String name; + + @Index + // Renaming this implicitly verifies that property names work for hibernate queries. + @Column(name = "some_value") + private int val; + + public TestEntity() {} + + public TestEntity(String name, int val) { + this.name = name; + this.val = val; + } + + public int getVal() { + return val; + } + + public String getName() { + return name; + } + } +} diff --git a/core/src/test/java/google/registry/reporting/spec11/Spec11EmailUtilsTest.java b/core/src/test/java/google/registry/reporting/spec11/Spec11EmailUtilsTest.java index c778b3f43..28014304a 100644 --- a/core/src/test/java/google/registry/reporting/spec11/Spec11EmailUtilsTest.java +++ b/core/src/test/java/google/registry/reporting/spec11/Spec11EmailUtilsTest.java @@ -17,11 +17,11 @@ package google.registry.reporting.spec11; import static com.google.common.truth.Truth.assertThat; import static google.registry.model.eppcommon.StatusValue.CLIENT_HOLD; import static google.registry.model.eppcommon.StatusValue.SERVER_HOLD; -import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.reporting.spec11.Spec11RegistrarThreatMatchesParserTest.getMatchA; import static google.registry.reporting.spec11.Spec11RegistrarThreatMatchesParserTest.getMatchB; import static google.registry.reporting.spec11.Spec11RegistrarThreatMatchesParserTest.sampleThreatMatches; import static google.registry.testing.DatabaseHelper.createTld; +import static google.registry.testing.DatabaseHelper.loadByEntity; import static google.registry.testing.DatabaseHelper.newDomainBase; import static google.registry.testing.DatabaseHelper.persistActiveHost; import static google.registry.testing.DatabaseHelper.persistResource; @@ -39,6 +39,8 @@ import google.registry.model.domain.DomainBase; import google.registry.model.host.HostResource; import google.registry.reporting.spec11.soy.Spec11EmailSoyInfo; import google.registry.testing.AppEngineExtension; +import google.registry.testing.DualDatabaseTest; +import google.registry.testing.TestOfyAndSql; import google.registry.util.EmailMessage; import google.registry.util.SendEmailService; import java.util.LinkedHashSet; @@ -48,11 +50,11 @@ import javax.mail.MessagingException; import javax.mail.internet.InternetAddress; import org.joda.time.LocalDate; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.mockito.ArgumentCaptor; /** Unit tests for {@link Spec11EmailUtils}. */ +@DualDatabaseTest class Spec11EmailUtilsTest { private static final ImmutableList FAKE_RESOURCES = ImmutableList.of("foo"); @@ -128,7 +130,7 @@ class Spec11EmailUtilsTest { persistDomainWithHost("c.com", host); } - @Test + @TestOfyAndSql void testSuccess_emailMonthlySpec11Reports() throws Exception { emailUtils.emailSpec11Reports( date, @@ -166,7 +168,7 @@ class Spec11EmailUtilsTest { Optional.empty()); } - @Test + @TestOfyAndSql void testSuccess_emailDailySpec11Reports() throws Exception { emailUtils.emailSpec11Reports( date, @@ -204,13 +206,11 @@ class Spec11EmailUtilsTest { Optional.empty()); } - @Test + @TestOfyAndSql void testSuccess_skipsInactiveDomain() throws Exception { // CLIENT_HOLD and SERVER_HOLD mean no DNS so we don't need to email it out - persistResource( - ofy().load().entity(aDomain).now().asBuilder().addStatusValue(SERVER_HOLD).build()); - persistResource( - ofy().load().entity(bDomain).now().asBuilder().addStatusValue(CLIENT_HOLD).build()); + persistResource(loadByEntity(aDomain).asBuilder().addStatusValue(SERVER_HOLD).build()); + persistResource(loadByEntity(bDomain).asBuilder().addStatusValue(CLIENT_HOLD).build()); emailUtils.emailSpec11Reports( date, Spec11EmailSoyInfo.MONTHLY_SPEC_11_EMAIL, @@ -237,7 +237,7 @@ class Spec11EmailUtilsTest { Optional.empty()); } - @Test + @TestOfyAndSql void testOneFailure_sendsAlert() throws Exception { // If there is one failure, we should still send the other message and then an alert email LinkedHashSet matches = new LinkedHashSet<>(); @@ -292,7 +292,7 @@ class Spec11EmailUtilsTest { Optional.empty()); } - @Test + @TestOfyAndSql void testSuccess_sendAlertEmail() throws Exception { emailUtils.sendAlertEmail("Spec11 Pipeline Alert: 2018-07", "Alert!"); verify(emailService).sendEmail(contentCaptor.capture()); @@ -306,7 +306,7 @@ class Spec11EmailUtilsTest { Optional.empty()); } - @Test + @TestOfyAndSql void testSuccess_useWhoisAbuseEmailIfAvailable() throws Exception { // if John Doe is the whois abuse contact, email them instead of the regular email persistResource( @@ -325,7 +325,7 @@ class Spec11EmailUtilsTest { .containsExactly(new InternetAddress("johndoe@theregistrar.com")); } - @Test + @TestOfyAndSql void testFailure_badClientId() { RuntimeException thrown = assertThrows(