zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-07-14 06:55:20 +02:00
Code Issues Projects Releases Packages Wiki Activity

Fix bug that was incorrectly assuming Cursor would always exist (#1088)

Browse source 
* Fix bug that was incorrectly assuming Cursor would always exist

In fact, the Cursor entity does not always exist (i.e. if an upload has never
previously been done on this TLD, i.e. it's a new TLD), and the code needs to be
resilient to its non-existence.

This bug was introduced in #1044.
This commit is contained in:
Ben McIlwain 2021-04-15 17:03:25 -04:00 • committed by GitHub
parent 13fd9971a9
commit 1adcd87de1
7 changed files with 49 additions and 10 deletions
Download patch file Download diff file Expand all files Collapse all files

5
core/src/main/java/google/registry/model/common/Cursor.java
View file

@ -36,6 +36,7 @@ import google.registry.persistence.VKey;
import google.registry.schema.replay.DatastoreAndSqlEntity;
import java.io.Serializable;
import java.util.List;
import java.util.Optional;
import javax.persistence.Column;
import javax.persistence.EnumType;
import javax.persistence.Enumerated;
@ -280,8 +281,8 @@ public class Cursor extends ImmutableObject implements DatastoreAndSqlEntity {
/**
* Returns the current time for a given cursor, or {@code START_OF_TIME} if the cursor is null.
*/
public static DateTime getCursorTimeOrStartOfTime(Cursor cursor) {
return cursor != null ? cursor.getCursorTime() : START_OF_TIME;
public static DateTime getCursorTimeOrStartOfTime(Optional<Cursor> cursor) {
return cursor.map(Cursor::getCursorTime).orElse(START_OF_TIME);
}
public DateTime getCursorTime() {

6
core/src/main/java/google/registry/rde/RdeReportAction.java
View file

@ -41,6 +41,7 @@ import google.registry.request.Response;
import google.registry.request.auth.Auth;
import java.io.IOException;
import java.io.InputStream;
import java.util.Optional;
import javax.inject.Inject;
import org.bouncycastle.openpgp.PGPPrivateKey;
import org.joda.time.DateTime;
@ -76,8 +77,9 @@ public final class RdeReportAction implements Runnable, EscrowTask {
@Override
public void runWithLock(DateTime watermark) throws Exception {
Cursor cursor =
transactIfJpaTm(() -> tm().loadByKey(Cursor.createVKey(CursorType.RDE_UPLOAD, tld)));
Optional<Cursor> cursor =
transactIfJpaTm(
() -> tm().loadByKeyIfPresent(Cursor.createVKey(CursorType.RDE_UPLOAD, tld)));
DateTime cursorTime = getCursorTimeOrStartOfTime(cursor);
if (isBeforeOrAt(cursorTime, watermark)) {
throw new NoContentException(

8
core/src/main/java/google/registry/rde/RdeStagingReducer.java
View file

@ -20,8 +20,8 @@ import static com.google.appengine.tools.cloudstorage.GcsServiceFactory.createGc
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.base.Verify.verify;
import static google.registry.model.common.Cursor.getCursorTimeOrStartOfTime;
import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.appengine.tools.cloudstorage.GcsFilename;
@ -210,7 +210,11 @@ public final class RdeStagingReducer extends Reducer<PendingDeposit, DepositFrag
tm().transact(
() -> {
Registry registry = Registry.get(tld);
Cursor cursor = ofy().load().key(Cursor.createKey(key.cursor(), registry)).now();
Optional<Cursor> cursor =
transactIfJpaTm(
() ->
tm().loadByKeyIfPresent(
Cursor.createVKey(key.cursor(), registry.getTldStr())));
DateTime position = getCursorTimeOrStartOfTime(cursor);
checkState(key.interval() != null, "Interval must be present");
DateTime newPosition = key.watermark().plus(key.interval());

4
core/src/main/java/google/registry/rde/RdeUploadAction.java
View file

@ -64,6 +64,7 @@ import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.URI;
import java.util.Optional;
import javax.inject.Inject;
import javax.inject.Named;
import org.bouncycastle.openpgp.PGPKeyPair;
@ -133,7 +134,8 @@ public final class RdeUploadAction implements Runnable, EscrowTask {
@Override
public void runWithLock(final DateTime watermark) throws Exception {
logger.atInfo().log("Verifying readiness to upload the RDE deposit.");
Cursor cursor = transactIfJpaTm(() -> tm().loadByKey(Cursor.createVKey(RDE_STAGING, tld)));
Optional<Cursor> cursor =
transactIfJpaTm(() -> tm().loadByKeyIfPresent(Cursor.createVKey(RDE_STAGING, tld)));
DateTime stagingCursorTime = getCursorTimeOrStartOfTime(cursor);
if (isBeforeOrAt(stagingCursorTime, watermark)) {
throw new NoContentException(

5
core/src/main/java/google/registry/reporting/icann/IcannReportingUploadAction.java
View file

@ -16,7 +16,6 @@ package google.registry.reporting.icann;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8;
import static google.registry.model.common.Cursor.getCursorTimeOrStartOfTime;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.request.Action.Method.POST;
import static javax.servlet.http.HttpServletResponse.SC_OK;
@ -107,10 +106,10 @@ public final class IcannReportingUploadAction implements Runnable {
// If cursor time is before now, upload the corresponding report
cursors.entrySet().stream()
.filter(entry -> getCursorTimeOrStartOfTime(entry.getKey()).isBefore(clock.nowUtc()))
.filter(entry -> entry.getKey().getCursorTime().isBefore(clock.nowUtc()))
.forEach(
entry -> {
DateTime cursorTime = getCursorTimeOrStartOfTime(entry.getKey());
DateTime cursorTime = entry.getKey().getCursorTime();
uploadReport(
cursorTime,
entry.getKey().getType(),

14
core/src/test/java/google/registry/rde/RdeReportActionTest.java
View file

@ -19,6 +19,7 @@ import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8;
import static com.google.common.truth.Truth.assertThat;
import static google.registry.model.common.Cursor.CursorType.RDE_REPORT;
import static google.registry.model.common.Cursor.CursorType.RDE_UPLOAD;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.testing.DatabaseHelper.createTld;
import static google.registry.testing.DatabaseHelper.loadByKey;
import static google.registry.testing.DatabaseHelper.persistResource;
@ -162,6 +163,19 @@ public class RdeReportActionTest {
assertThat(report.getWatermark()).isEqualTo(DateTime.parse("2010-10-17T00:00:00Z"));
}
@TestOfyAndSql
void testRunWithLock_nonexistentCursor_throws204() {
tm().transact(() -> tm().delete(Cursor.createVKey(RDE_UPLOAD, "test")));
NoContentException thrown =
assertThrows(
NoContentException.class, () -> createAction().runWithLock(loadRdeReportCursor()));
assertThat(thrown)
.hasMessageThat()
.isEqualTo(
"Waiting on RdeUploadAction for TLD test to send 2006-06-06T00:00:00.000Z report; last"
+ " upload completion was at 1970-01-01T00:00:00.000Z");
}
@TestOfyAndSql
void testRunWithLock_uploadNotFinished_throws204() {
persistResource(

17
core/src/test/java/google/registry/rde/RdeUploadActionTest.java
View file

@ -318,6 +318,23 @@ public class RdeUploadActionTest {
assertThat(stderr).contains("rde-unittest@registry.test");
}
@TestOfyAndSql
void testRunWithLock_nonexistentCursor_throws204() throws Exception {
int port = sftpd.serve("user", "password", folder);
URI uploadUrl = URI.create(String.format("sftp://user:password@localhost:%d/", port));
DateTime uploadCursor = DateTime.parse("2010-10-17TZ");
RdeUploadAction action = createAction(uploadUrl);
NoContentException thrown =
assertThrows(NoContentException.class, () -> action.runWithLock(uploadCursor));
assertThat(thrown)
.hasMessageThat()
.isEqualTo(
"Waiting on RdeStagingAction for TLD tld to send 2010-10-17T00:00:00.000Z upload; last"
+ " RDE staging completion was at 1970-01-01T00:00:00.000Z");
assertNoTasksEnqueued("rde-upload");
assertThat(folder.list()).isEmpty();
}
@TestOfyAndSql
void testRunWithLock_stagingNotFinished_throws204() {
URI url = URI.create("sftp://user:password@localhost:32323/");