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).
This commit is contained in:
Michael Muller 2021-04-16 12:21:03 -04:00 committed by GitHub
parent bc2a5dbc02
commit 1c96cd64fe
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 600 additions and 28 deletions

View file

@ -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;
}
}
}

View file

@ -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<String> 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<RegistrarThreatMatches> 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(