From d65bf2a714361ea30c8f79b3accda3857d74bcbe Mon Sep 17 00:00:00 2001 From: mcilwain Date: Fri, 22 Apr 2016 08:04:56 -0700 Subject: [PATCH] Daggerize/Actionize the load snapshot servlet This is needed to eliminate the last non-injected use of BigqueryFactory. It gets rid of the deprecated HttpServletUtils now that we don't have lots of separate servlets, and changes the check snapshot servlet's usage of such. It also fixes up some related minor style issues in the update snapshot action (and its injectable parameters). ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=120547998 --- .../registry/bigquery/BigqueryFactory.java | 2 +- .../registry/config/RegistryConfig.java | 14 -- .../registry/config/TestRegistryConfig.java | 10 - .../registry/export/CheckSnapshotServlet.java | 55 +++--- .../registry/export/ExportRequestModule.java | 45 +++-- ...otServlet.java => LoadSnapshotAction.java} | 144 +++++--------- .../export/UpdateSnapshotViewAction.java | 39 ++-- .../backend/BackendRequestComponent.java | 2 + .../registry/request/RequestHandler.java | 5 +- .../registry/util/HttpServletUtils.java | 48 ----- .../export/CheckSnapshotServletTest.java | 97 +++++----- ...tTest.java => LoadSnapshotActionTest.java} | 176 ++++++------------ .../export/UpdateSnapshotViewActionTest.java | 30 +-- 13 files changed, 248 insertions(+), 419 deletions(-) rename java/com/google/domain/registry/export/{LoadSnapshotServlet.java => LoadSnapshotAction.java} (50%) delete mode 100644 java/com/google/domain/registry/util/HttpServletUtils.java rename javatests/com/google/domain/registry/export/{LoadSnapshotServletTest.java => LoadSnapshotActionTest.java} (54%) diff --git a/java/com/google/domain/registry/bigquery/BigqueryFactory.java b/java/com/google/domain/registry/bigquery/BigqueryFactory.java index f428696fb..803139389 100644 --- a/java/com/google/domain/registry/bigquery/BigqueryFactory.java +++ b/java/com/google/domain/registry/bigquery/BigqueryFactory.java @@ -59,7 +59,7 @@ public class BigqueryFactory { @VisibleForTesting Subfactory subfactory = new Subfactory(); - @Inject public BigqueryFactory() {} + @Inject BigqueryFactory() {} /** This class is broken out solely so that it can be mocked inside of tests. */ static class Subfactory { diff --git a/java/com/google/domain/registry/config/RegistryConfig.java b/java/com/google/domain/registry/config/RegistryConfig.java index 639ea71df..d30f0ff36 100644 --- a/java/com/google/domain/registry/config/RegistryConfig.java +++ b/java/com/google/domain/registry/config/RegistryConfig.java @@ -51,20 +51,6 @@ public interface RegistryConfig { */ public String getDomainListsBucket(); - /** - * Returns the BigQuery dataset for storing directly imported datastore snapshots. - * - * @see com.google.domain.registry.export.LoadSnapshotServlet - */ - public String getSnapshotsDataset(); - - /** - * Returns the BigQuery dataset for storing views pointing to the latest datastore snapshot. - * - * @see com.google.domain.registry.export.UpdateSnapshotViewAction - */ - public String getLatestSnapshotDataset(); - /** * Number of sharded commit log buckets. * diff --git a/java/com/google/domain/registry/config/TestRegistryConfig.java b/java/com/google/domain/registry/config/TestRegistryConfig.java index ae5536198..8371f4c1f 100644 --- a/java/com/google/domain/registry/config/TestRegistryConfig.java +++ b/java/com/google/domain/registry/config/TestRegistryConfig.java @@ -57,16 +57,6 @@ public class TestRegistryConfig implements RegistryConfig { return getProjectId() + "-domain-lists"; } - @Override - public String getSnapshotsDataset() { - return "snapshots"; - } - - @Override - public String getLatestSnapshotDataset() { - return "latest_snapshot"; - } - @Override public String getCommitsBucket() { return getProjectId() + "-commits"; diff --git a/java/com/google/domain/registry/export/CheckSnapshotServlet.java b/java/com/google/domain/registry/export/CheckSnapshotServlet.java index dc1771746..278b79b9c 100644 --- a/java/com/google/domain/registry/export/CheckSnapshotServlet.java +++ b/java/com/google/domain/registry/export/CheckSnapshotServlet.java @@ -15,8 +15,11 @@ package com.google.domain.registry.export; import static com.google.common.base.MoreObjects.firstNonNull; +import static com.google.common.collect.Sets.intersection; import static com.google.common.html.HtmlEscapers.htmlEscaper; -import static com.google.domain.registry.util.HttpServletUtils.getRequiredParameterValue; +import static com.google.domain.registry.export.LoadSnapshotAction.enqueueLoadSnapshotTask; +import static com.google.domain.registry.request.RequestParameters.extractRequiredParameter; +import static com.google.domain.registry.util.FormattingLogger.getLoggerForCallerClass; import static javax.servlet.http.HttpServletResponse.SC_ACCEPTED; import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; @@ -28,12 +31,12 @@ import com.google.appengine.api.taskqueue.TaskHandle; import com.google.appengine.api.taskqueue.TaskOptions; import com.google.appengine.api.taskqueue.TaskOptions.Method; import com.google.common.base.Joiner; -import com.google.common.base.Optional; import com.google.common.base.Splitter; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import com.google.common.net.MediaType; import com.google.domain.registry.export.DatastoreBackupInfo.BackupStatus; +import com.google.domain.registry.request.HttpException.BadRequestException; import com.google.domain.registry.util.FormattingLogger; import com.google.domain.registry.util.NonFinalForTesting; @@ -42,6 +45,7 @@ import org.joda.time.PeriodType; import org.joda.time.format.PeriodFormat; import java.io.IOException; +import java.util.Set; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; @@ -62,14 +66,11 @@ public class CheckSnapshotServlet extends HttpServlet { /** The maximum amount of time we allow a backup to run before abandoning it. */ static final Duration MAXIMUM_BACKUP_RUNNING_TIME = Duration.standardHours(20); - private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); + private static final FormattingLogger logger = getLoggerForCallerClass(); @NonFinalForTesting private static DatastoreBackupService backupService = DatastoreBackupService.get(); - @NonFinalForTesting - private static LoadSnapshotServlet loadSnapshotServlet = new LoadSnapshotServlet(); - @Override public void service(HttpServletRequest req, HttpServletResponse rsp) throws IOException { try { @@ -87,18 +88,27 @@ public class CheckSnapshotServlet extends HttpServlet { @Override public void doGet(HttpServletRequest req, HttpServletResponse rsp) throws IOException { - String snapshotName = getRequiredParameterValue(req, SNAPSHOT_NAME_PARAM); - rsp.getWriter().write(backupService.findByName(snapshotName).getInformation()); + // TODO(b/28266757): Remove this try/catch/rethrow block once this servlet is Daggerized. + try { + String snapshotName = extractRequiredParameter(req, SNAPSHOT_NAME_PARAM); + rsp.getWriter().write(backupService.findByName(snapshotName).getInformation()); + } catch (BadRequestException e) { + throw new IllegalArgumentException(e.getMessage()); + } } @Override public void doPost(HttpServletRequest req, HttpServletResponse rsp) throws IOException { - String snapshotName = getRequiredParameterValue(req, SNAPSHOT_NAME_PARAM); - // TODO(b/19237926): make this non-optional once all new tasks will have this parameter. - String kindsToLoadParam = req.getParameter(SNAPSHOT_KINDS_TO_LOAD_PARAM); - Optional> kindsToLoad = Optional.fromNullable( - kindsToLoadParam == null ? null - : ImmutableSet.copyOf(Splitter.on(',').split(kindsToLoadParam))); + String snapshotName; + String kindsToLoadParam; + // TODO(b/28266757): Remove this try/catch/rethrow block once this servlet is Daggerized. + try { + snapshotName = extractRequiredParameter(req, SNAPSHOT_NAME_PARAM); + kindsToLoadParam = extractRequiredParameter(req, SNAPSHOT_KINDS_TO_LOAD_PARAM); + } catch (BadRequestException e) { + throw new IllegalArgumentException(e.getMessage()); + } + Set kindsToLoad = ImmutableSet.copyOf(Splitter.on(',').split(kindsToLoadParam)); // Look up the backup by the provided name, stopping if we can't find it. DatastoreBackupInfo backup; @@ -137,21 +147,20 @@ public class CheckSnapshotServlet extends HttpServlet { String snapshotId = snapshotName.startsWith(ExportSnapshotServlet.SNAPSHOT_PREFIX) ? snapshotName.substring(ExportSnapshotServlet.SNAPSHOT_PREFIX.length()) : backup.getStartTime().toString("YYYYMMdd_HHmmss"); - // Log a warning if kindsToLoad is specified and not a subset of the exported snapshot kinds. - if (kindsToLoad.isPresent() && !backup.getKinds().containsAll(kindsToLoad.get())) { - logger.warningfmt("Kinds to load included non-exported kinds: %s", - Sets.difference(kindsToLoad.get(), backup.getKinds())); + // Log a warning if kindsToLoad is not a subset of the exported snapshot kinds. + if (!backup.getKinds().containsAll(kindsToLoad)) { + logger.warningfmt( + "Kinds to load included non-exported kinds: %s", + Sets.difference(kindsToLoad, backup.getKinds())); } // Load kinds from the snapshot, limited to those also in kindsToLoad (if it's present). - ImmutableSet exportedKindsToLoad = ImmutableSet.copyOf(kindsToLoad.isPresent() - ? Sets.intersection(backup.getKinds(), kindsToLoad.get()) - : backup.getKinds()); + ImmutableSet exportedKindsToLoad = + ImmutableSet.copyOf(intersection(backup.getKinds(), kindsToLoad)); String message = String.format("Datastore backup %s complete - ", snapshotName); if (exportedKindsToLoad.isEmpty()) { message += "no kinds to load into BigQuery"; } else { - loadSnapshotServlet.enqueueLoadTask( - snapshotId, backup.getGcsFilename().get(), exportedKindsToLoad); + enqueueLoadSnapshotTask(snapshotId, backup.getGcsFilename().get(), exportedKindsToLoad); message += "BigQuery load task enqueued"; } logger.info(message); diff --git a/java/com/google/domain/registry/export/ExportRequestModule.java b/java/com/google/domain/registry/export/ExportRequestModule.java index 12f87e5c5..7e867b108 100644 --- a/java/com/google/domain/registry/export/ExportRequestModule.java +++ b/java/com/google/domain/registry/export/ExportRequestModule.java @@ -17,9 +17,12 @@ package com.google.domain.registry.export; import static com.google.domain.registry.export.BigqueryPollJobAction.CHAINED_TASK_QUEUE_HEADER; import static com.google.domain.registry.export.BigqueryPollJobAction.JOB_ID_HEADER; import static com.google.domain.registry.export.BigqueryPollJobAction.PROJECT_ID_HEADER; -import static com.google.domain.registry.export.UpdateSnapshotViewAction.SNAPSHOT_DATASET_ID_PARAM; -import static com.google.domain.registry.export.UpdateSnapshotViewAction.SNAPSHOT_KIND_PARAM; -import static com.google.domain.registry.export.UpdateSnapshotViewAction.SNAPSHOT_TABLE_ID_PARAM; +import static com.google.domain.registry.export.LoadSnapshotAction.LOAD_SNAPSHOT_FILE_PARAM; +import static com.google.domain.registry.export.LoadSnapshotAction.LOAD_SNAPSHOT_ID_PARAM; +import static com.google.domain.registry.export.LoadSnapshotAction.LOAD_SNAPSHOT_KINDS_PARAM; +import static com.google.domain.registry.export.UpdateSnapshotViewAction.UPDATE_SNAPSHOT_DATASET_ID_PARAM; +import static com.google.domain.registry.export.UpdateSnapshotViewAction.UPDATE_SNAPSHOT_KIND_PARAM; +import static com.google.domain.registry.export.UpdateSnapshotViewAction.UPDATE_SNAPSHOT_TABLE_ID_PARAM; import static com.google.domain.registry.request.RequestParameters.extractRequiredHeader; import com.google.domain.registry.request.Header; @@ -35,21 +38,39 @@ import javax.servlet.http.HttpServletRequest; public final class ExportRequestModule { @Provides - @Parameter(SNAPSHOT_DATASET_ID_PARAM) - static String provideDatasetId(HttpServletRequest req) { - return extractRequiredHeader(req, SNAPSHOT_DATASET_ID_PARAM); + @Parameter(UPDATE_SNAPSHOT_DATASET_ID_PARAM) + static String provideUpdateSnapshotDatasetId(HttpServletRequest req) { + return extractRequiredHeader(req, UPDATE_SNAPSHOT_DATASET_ID_PARAM); } @Provides - @Parameter(SNAPSHOT_TABLE_ID_PARAM) - static String provideTableId(HttpServletRequest req) { - return extractRequiredHeader(req, SNAPSHOT_TABLE_ID_PARAM); + @Parameter(UPDATE_SNAPSHOT_TABLE_ID_PARAM) + static String provideUpdateSnapshotTableId(HttpServletRequest req) { + return extractRequiredHeader(req, UPDATE_SNAPSHOT_TABLE_ID_PARAM); } @Provides - @Parameter(SNAPSHOT_KIND_PARAM) - static String provideKind(HttpServletRequest req) { - return extractRequiredHeader(req, SNAPSHOT_KIND_PARAM); + @Parameter(UPDATE_SNAPSHOT_KIND_PARAM) + static String provideUpdateSnapshotKind(HttpServletRequest req) { + return extractRequiredHeader(req, UPDATE_SNAPSHOT_KIND_PARAM); + } + + @Provides + @Parameter(LOAD_SNAPSHOT_FILE_PARAM) + static String provideLoadSnapshotFile(HttpServletRequest req) { + return extractRequiredHeader(req, LOAD_SNAPSHOT_FILE_PARAM); + } + + @Provides + @Parameter(LOAD_SNAPSHOT_ID_PARAM) + static String provideLoadSnapshotId(HttpServletRequest req) { + return extractRequiredHeader(req, LOAD_SNAPSHOT_ID_PARAM); + } + + @Provides + @Parameter(LOAD_SNAPSHOT_KINDS_PARAM) + static String provideLoadSnapshotKinds(HttpServletRequest req) { + return extractRequiredHeader(req, LOAD_SNAPSHOT_KINDS_PARAM); } @Provides diff --git a/java/com/google/domain/registry/export/LoadSnapshotServlet.java b/java/com/google/domain/registry/export/LoadSnapshotAction.java similarity index 50% rename from java/com/google/domain/registry/export/LoadSnapshotServlet.java rename to java/com/google/domain/registry/export/LoadSnapshotAction.java index 0f30c0d47..45e2b0c63 100644 --- a/java/com/google/domain/registry/export/LoadSnapshotServlet.java +++ b/java/com/google/domain/registry/export/LoadSnapshotAction.java @@ -14,27 +14,19 @@ package com.google.domain.registry.export; +import static com.google.appengine.api.taskqueue.QueueFactory.getQueue; import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.html.HtmlEscapers.htmlEscaper; -import static com.google.domain.registry.util.HttpServletUtils.getRequiredParameterValue; -import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; -import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; -import static javax.servlet.http.HttpServletResponse.SC_OK; +import static com.google.domain.registry.export.UpdateSnapshotViewAction.createViewUpdateTask; +import static com.google.domain.registry.request.Action.Method.POST; +import static com.google.domain.registry.util.FormattingLogger.getLoggerForCallerClass; -import com.google.api.client.extensions.appengine.http.UrlFetchTransport; -import com.google.api.client.googleapis.extensions.appengine.auth.oauth2.AppIdentityCredential; -import com.google.api.client.json.jackson2.JacksonFactory; import com.google.api.services.bigquery.Bigquery; -import com.google.api.services.bigquery.BigqueryScopes; -import com.google.api.services.bigquery.model.Dataset; -import com.google.api.services.bigquery.model.DatasetReference; import com.google.api.services.bigquery.model.Job; import com.google.api.services.bigquery.model.JobConfiguration; import com.google.api.services.bigquery.model.JobConfigurationLoad; import com.google.api.services.bigquery.model.JobReference; import com.google.api.services.bigquery.model.TableReference; -import com.google.appengine.api.taskqueue.QueueFactory; import com.google.appengine.api.taskqueue.TaskHandle; import com.google.appengine.api.taskqueue.TaskOptions; import com.google.appengine.api.taskqueue.TaskOptions.Method; @@ -42,99 +34,82 @@ import com.google.common.base.Joiner; import com.google.common.base.Splitter; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.google.common.net.MediaType; import com.google.domain.registry.bigquery.BigqueryFactory; -import com.google.domain.registry.bigquery.BigqueryJobFailureException; import com.google.domain.registry.bigquery.BigqueryUtils.SourceFormat; import com.google.domain.registry.bigquery.BigqueryUtils.WriteDisposition; -import com.google.domain.registry.config.RegistryEnvironment; +import com.google.domain.registry.config.ConfigModule.Config; import com.google.domain.registry.export.BigqueryPollJobAction.BigqueryPollJobEnqueuer; +import com.google.domain.registry.request.Action; +import com.google.domain.registry.request.HttpException.BadRequestException; +import com.google.domain.registry.request.HttpException.InternalServerErrorException; +import com.google.domain.registry.request.Parameter; import com.google.domain.registry.util.Clock; import com.google.domain.registry.util.FormattingLogger; -import com.google.domain.registry.util.NonFinalForTesting; -import com.google.domain.registry.util.Retrier; -import com.google.domain.registry.util.SystemClock; -import com.google.domain.registry.util.SystemSleeper; -import com.google.domain.registry.util.TaskEnqueuer; import org.joda.time.DateTime; import java.io.IOException; -import javax.servlet.http.HttpServlet; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; +import javax.inject.Inject; -/** Load a datastore snapshot from Google Cloud Storage into BigQuery. */ -public class LoadSnapshotServlet extends HttpServlet { - - private static final RegistryEnvironment ENVIRONMENT = RegistryEnvironment.get(); +/** Action to load a Datastore snapshot from Google Cloud Storage into BigQuery. */ +@Action(path = LoadSnapshotAction.PATH, method = POST) +public class LoadSnapshotAction implements Runnable { /** Parameter names for passing parameters into the servlet. */ - static final String SNAPSHOT_ID_PARAM = "id"; - static final String SNAPSHOT_FILE_PARAM = "file"; - static final String SNAPSHOT_KINDS_PARAM = "kinds"; + static final String LOAD_SNAPSHOT_ID_PARAM = "id"; + static final String LOAD_SNAPSHOT_FILE_PARAM = "file"; + static final String LOAD_SNAPSHOT_KINDS_PARAM = "kinds"; + + static final String SNAPSHOTS_DATASET = "snapshots"; /** Servlet-specific details needed for enqueuing tasks against itself. */ static final String QUEUE = "export-snapshot"; // See queue.xml. static final String PATH = "/_dr/task/loadSnapshot"; // See web.xml. - private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); + private static final FormattingLogger logger = getLoggerForCallerClass(); - @NonFinalForTesting - private static Clock clock = new SystemClock(); + @Inject BigqueryFactory bigqueryFactory; + @Inject BigqueryPollJobEnqueuer bigqueryPollEnqueuer; + @Inject Clock clock; + @Inject @Config("projectId") String projectId; + @Inject @Parameter(LOAD_SNAPSHOT_FILE_PARAM) String snapshotFile; + @Inject @Parameter(LOAD_SNAPSHOT_ID_PARAM) String snapshotId; + @Inject @Parameter(LOAD_SNAPSHOT_KINDS_PARAM) String snapshotKinds; + @Inject LoadSnapshotAction() {} - @NonFinalForTesting - private static BigqueryFactory bigqueryFactory = new BigqueryFactory(); - - @NonFinalForTesting - private static BigqueryPollJobEnqueuer bigqueryPollEnqueuer = new BigqueryPollJobEnqueuer( - new TaskEnqueuer(new Retrier(new SystemSleeper(), 5))); - - /** Enqueue a task for starting a backup load. Not static for better testability. */ - public TaskHandle enqueueLoadTask( + /** Enqueue a task for starting a backup load. */ + public static TaskHandle enqueueLoadSnapshotTask( String snapshotId, String gcsFile, ImmutableSet kinds) { - return QueueFactory.getQueue(QUEUE).add( + return getQueue(QUEUE).add( TaskOptions.Builder.withUrl(PATH) .method(Method.POST) - .param(SNAPSHOT_ID_PARAM, snapshotId) - .param(SNAPSHOT_FILE_PARAM, gcsFile) - .param(SNAPSHOT_KINDS_PARAM, Joiner.on(',').join(kinds))); + .param(LOAD_SNAPSHOT_ID_PARAM, snapshotId) + .param(LOAD_SNAPSHOT_FILE_PARAM, gcsFile) + .param(LOAD_SNAPSHOT_KINDS_PARAM, Joiner.on(',').join(kinds))); } @Override - public void doPost(HttpServletRequest req, HttpServletResponse rsp) throws IOException { + public void run() { try { - String snapshotId = getRequiredParameterValue(req, SNAPSHOT_ID_PARAM); - String gcsFilename = getRequiredParameterValue(req, SNAPSHOT_FILE_PARAM); - ImmutableList kinds = ImmutableList.copyOf( - Splitter.on(',').splitToList(getRequiredParameterValue(req, SNAPSHOT_KINDS_PARAM))); - - String message = loadSnapshot(snapshotId, gcsFilename, kinds); - - rsp.setStatus(SC_OK); - rsp.setContentType(MediaType.PLAIN_TEXT_UTF_8.toString()); - rsp.getWriter().write("OK\n\n" + message); + String message = + loadSnapshot(snapshotId, snapshotFile, Splitter.on(',').split(snapshotKinds)); + logger.infofmt("Loaded snapshot successfully: %s", message); } catch (Throwable e) { - logger.severe(e, e.toString()); - rsp.sendError( - e instanceof IllegalArgumentException ? SC_BAD_REQUEST : SC_INTERNAL_SERVER_ERROR, - htmlEscaper().escape(firstNonNull(e.getMessage(), e.toString()))); + logger.severe(e, "Error loading snapshot"); + if (e instanceof IllegalArgumentException) { + throw new BadRequestException("Error calling load snapshot: " + e.getMessage(), e); + } else { + throw new InternalServerErrorException( + "Error loading snapshot: " + firstNonNull(e.getMessage(), e.toString())); + } } } - private String loadSnapshot(String snapshotId, String gcsFilename, ImmutableList kinds) + private String loadSnapshot(String snapshotId, String gcsFilename, Iterable kinds) throws IOException { - Bigquery bigquery = bigqueryFactory.create( - getClass().getSimpleName(), - new UrlFetchTransport(), - new JacksonFactory(), - new AppIdentityCredential(BigqueryScopes.all())); - String projectId = ENVIRONMENT.config().getProjectId(); + Bigquery bigquery = bigqueryFactory.create(projectId, SNAPSHOTS_DATASET); DateTime now = clock.nowUtc(); - - ensureDataset(bigquery, projectId, ENVIRONMENT.config().getSnapshotsDataset()); - String loadMessage = String.format("Loading datastore snapshot %s from %s...", snapshotId, gcsFilename); logger.info(loadMessage); @@ -155,9 +130,8 @@ public class LoadSnapshotServlet extends HttpServlet { // well-known view in BigQuery to point at the newly loaded snapshot table for this kind. bigqueryPollEnqueuer.enqueuePollTask( jobRef, - UpdateSnapshotViewAction.createViewUpdateTask( - ENVIRONMENT.config().getSnapshotsDataset(), tableId, kindName), - QueueFactory.getQueue(UpdateSnapshotViewAction.QUEUE)); + createViewUpdateTask(SNAPSHOTS_DATASET, tableId, kindName), + getQueue(UpdateSnapshotViewAction.QUEUE)); builder.append(String.format(" - %s:%s\n", projectId, jobId)); logger.infofmt("Submitted load job %s:%s", projectId, jobId); @@ -165,24 +139,6 @@ public class LoadSnapshotServlet extends HttpServlet { return builder.toString(); } - private static void ensureDataset(Bigquery bigquery, String projectId, String datasetId) - throws IOException { - try { - bigquery.datasets() - .insert(projectId, - new Dataset().setDatasetReference( - new DatasetReference() - .setProjectId(projectId) - .setDatasetId(datasetId))) - .execute(); - } catch (IOException e) { - // Swallow errors about a duplicate dataset, and throw any other ones. - if (!BigqueryJobFailureException.create(e).getReason().equals("duplicate")) { - throw e; - } - } - } - private static String getBackupInfoFileForKind(String backupInfoFile, String kindName) { String extension = ".backup_info"; checkArgument(backupInfoFile.endsWith(extension), "backup info file extension missing"); @@ -190,10 +146,10 @@ public class LoadSnapshotServlet extends HttpServlet { return Joiner.on('.').join(prefix, kindName, extension.substring(1)); } - private static Job makeLoadJob(JobReference jobRef, String sourceUri, String tableId) { + private Job makeLoadJob(JobReference jobRef, String sourceUri, String tableId) { TableReference tableReference = new TableReference() .setProjectId(jobRef.getProjectId()) - .setDatasetId(ENVIRONMENT.config().getSnapshotsDataset()) + .setDatasetId(SNAPSHOTS_DATASET) .setTableId(tableId); return new Job() .setJobReference(jobRef) diff --git a/java/com/google/domain/registry/export/UpdateSnapshotViewAction.java b/java/com/google/domain/registry/export/UpdateSnapshotViewAction.java index 8fa245fef..eaf46a8f7 100644 --- a/java/com/google/domain/registry/export/UpdateSnapshotViewAction.java +++ b/java/com/google/domain/registry/export/UpdateSnapshotViewAction.java @@ -23,7 +23,7 @@ import com.google.api.services.bigquery.model.ViewDefinition; import com.google.appengine.api.taskqueue.TaskOptions; import com.google.appengine.api.taskqueue.TaskOptions.Method; import com.google.domain.registry.bigquery.BigqueryFactory; -import com.google.domain.registry.config.RegistryEnvironment; +import com.google.domain.registry.config.ConfigModule.Config; import com.google.domain.registry.request.Action; import com.google.domain.registry.request.HttpException.InternalServerErrorException; import com.google.domain.registry.request.Parameter; @@ -38,12 +38,12 @@ import javax.inject.Inject; @Action(path = UpdateSnapshotViewAction.PATH, method = POST) public class UpdateSnapshotViewAction implements Runnable { - private static final RegistryEnvironment ENVIRONMENT = RegistryEnvironment.get(); - /** Headers for passing parameters into the servlet. */ - static final String SNAPSHOT_DATASET_ID_PARAM = "dataset"; - static final String SNAPSHOT_TABLE_ID_PARAM = "table"; - static final String SNAPSHOT_KIND_PARAM = "kind"; + static final String UPDATE_SNAPSHOT_DATASET_ID_PARAM = "dataset"; + static final String UPDATE_SNAPSHOT_TABLE_ID_PARAM = "table"; + static final String UPDATE_SNAPSHOT_KIND_PARAM = "kind"; + + static final String LATEST_SNAPSHOT_DATASET = "latest_snapshot"; /** Servlet-specific details needed for enqueuing tasks against itself. */ static final String QUEUE = "export-snapshot-update-view"; // See queue.xml. @@ -51,9 +51,10 @@ public class UpdateSnapshotViewAction implements Runnable { private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); - @Inject @Parameter(SNAPSHOT_DATASET_ID_PARAM) String datasetId; - @Inject @Parameter(SNAPSHOT_TABLE_ID_PARAM) String tableId; - @Inject @Parameter(SNAPSHOT_KIND_PARAM) String kindName; + @Inject @Parameter(UPDATE_SNAPSHOT_DATASET_ID_PARAM) String datasetId; + @Inject @Parameter(UPDATE_SNAPSHOT_TABLE_ID_PARAM) String tableId; + @Inject @Parameter(UPDATE_SNAPSHOT_KIND_PARAM) String kindName; + @Inject @Config("projectId") String projectId; @Inject BigqueryFactory bigqueryFactory; @Inject UpdateSnapshotViewAction() {} @@ -62,9 +63,9 @@ public class UpdateSnapshotViewAction implements Runnable { String datasetId, String tableId, String kindName) { return TaskOptions.Builder.withUrl(PATH) .method(Method.POST) - .param(SNAPSHOT_DATASET_ID_PARAM, datasetId) - .param(SNAPSHOT_TABLE_ID_PARAM, tableId) - .param(SNAPSHOT_KIND_PARAM, kindName); + .param(UPDATE_SNAPSHOT_DATASET_ID_PARAM, datasetId) + .param(UPDATE_SNAPSHOT_TABLE_ID_PARAM, tableId) + .param(UPDATE_SNAPSHOT_KIND_PARAM, kindName); } @Override @@ -79,14 +80,12 @@ public class UpdateSnapshotViewAction implements Runnable { private void updateSnapshotView(String datasetId, String tableId, String kindName) throws IOException { - String projectId = ENVIRONMENT.config().getProjectId(); - Bigquery bigquery = - bigqueryFactory.create(projectId, ENVIRONMENT.config().getLatestSnapshotDataset()); + Bigquery bigquery = bigqueryFactory.create(projectId, LATEST_SNAPSHOT_DATASET); updateTable(bigquery, new Table() .setTableReference(new TableReference() .setProjectId(projectId) - .setDatasetId(ENVIRONMENT.config().getLatestSnapshotDataset()) + .setDatasetId(LATEST_SNAPSHOT_DATASET) .setTableId(kindName)) .setView(new ViewDefinition().setQuery( SqlTemplate.create("SELECT * FROM [%DATASET%.%TABLE%]") @@ -94,10 +93,12 @@ public class UpdateSnapshotViewAction implements Runnable { .put("TABLE", tableId) .build()))); - String message = String.format( + logger.infofmt( "Updated view %s:%s to point at snapshot table %s:%s.", - ENVIRONMENT.config().getLatestSnapshotDataset(), kindName, datasetId, tableId); - logger.info(message); + LATEST_SNAPSHOT_DATASET, + kindName, + datasetId, + tableId); } private static void updateTable(Bigquery bigquery, Table table) throws IOException { diff --git a/java/com/google/domain/registry/module/backend/BackendRequestComponent.java b/java/com/google/domain/registry/module/backend/BackendRequestComponent.java index cbe89ac48..6249bc825 100644 --- a/java/com/google/domain/registry/module/backend/BackendRequestComponent.java +++ b/java/com/google/domain/registry/module/backend/BackendRequestComponent.java @@ -31,6 +31,7 @@ import com.google.domain.registry.export.BigqueryPollJobAction; import com.google.domain.registry.export.ExportDomainListsAction; import com.google.domain.registry.export.ExportRequestModule; import com.google.domain.registry.export.ExportReservedTermsAction; +import com.google.domain.registry.export.LoadSnapshotAction; import com.google.domain.registry.export.SyncGroupMembersAction; import com.google.domain.registry.export.UpdateSnapshotViewAction; import com.google.domain.registry.export.sheet.SheetModule; @@ -89,6 +90,7 @@ interface BackendRequestComponent { ExportCommitLogDiffAction exportCommitLogDiffAction(); ExportDomainListsAction exportDomainListsAction(); ExportReservedTermsAction exportReservedTermsAction(); + LoadSnapshotAction loadSnapshotAction(); MetricsExportAction metricsExportAction(); NordnUploadAction nordnUploadAction(); NordnVerifyAction nordnVerifyAction(); diff --git a/java/com/google/domain/registry/request/RequestHandler.java b/java/com/google/domain/registry/request/RequestHandler.java index 49af03976..3438e1eb9 100644 --- a/java/com/google/domain/registry/request/RequestHandler.java +++ b/java/com/google/domain/registry/request/RequestHandler.java @@ -17,9 +17,9 @@ package com.google.domain.registry.request; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Strings.nullToEmpty; import static com.google.common.net.HttpHeaders.LOCATION; +import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8; import static com.google.domain.registry.security.XsrfTokenManager.X_CSRF_TOKEN; import static com.google.domain.registry.security.XsrfTokenManager.validateToken; -import static com.google.domain.registry.util.HttpServletUtils.sendOk; import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN; import static javax.servlet.http.HttpServletResponse.SC_METHOD_NOT_ALLOWED; import static javax.servlet.http.HttpServletResponse.SC_MOVED_TEMPORARILY; @@ -149,7 +149,8 @@ public final class RequestHandler { try { route.get().instantiator().apply(component).run(); if (route.get().action().automaticallyPrintOk()) { - sendOk(rsp); + rsp.setContentType(PLAIN_TEXT_UTF_8.toString()); + rsp.getWriter().write("OK\n"); } } catch (HttpException e) { e.send(rsp); diff --git a/java/com/google/domain/registry/util/HttpServletUtils.java b/java/com/google/domain/registry/util/HttpServletUtils.java deleted file mode 100644 index 234d6a0e2..000000000 --- a/java/com/google/domain/registry/util/HttpServletUtils.java +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2016 The Domain Registry 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 com.google.domain.registry.util; - -import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8; -import static com.google.domain.registry.util.PreconditionsUtils.checkArgumentNotNull; - -import java.io.IOException; - -import javax.servlet.http.HttpServlet; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -/** Utility methods for working with {@link HttpServlet}-related classes. */ -public final class HttpServletUtils { - - /** - * Returns the value of the given request's first {@code name} parameter, or throws - * {@code IllegalArgumentException} if the parameter is not present. - * - * @deprecated in favor of RequestParameters.extractRequiredParameter - */ - @Deprecated - public static String getRequiredParameterValue(HttpServletRequest req, String name) { - return checkArgumentNotNull(req.getParameter(name), "Missing required parameter: %s", name); - } - - /** - * Sets the content type on an HttpServletResponse to UTF-8, returns 200 OK status, and sends - * "OK". - */ - public static void sendOk(HttpServletResponse rsp) throws IOException { - rsp.setContentType(PLAIN_TEXT_UTF_8.toString()); - rsp.getWriter().write("OK\n"); - } -} diff --git a/javatests/com/google/domain/registry/export/CheckSnapshotServletTest.java b/javatests/com/google/domain/registry/export/CheckSnapshotServletTest.java index aeecd154c..d18867a41 100644 --- a/javatests/com/google/domain/registry/export/CheckSnapshotServletTest.java +++ b/javatests/com/google/domain/registry/export/CheckSnapshotServletTest.java @@ -15,6 +15,9 @@ package com.google.domain.registry.export; import static com.google.common.truth.Truth.assertThat; +import static com.google.domain.registry.export.CheckSnapshotServlet.SNAPSHOT_KINDS_TO_LOAD_PARAM; +import static com.google.domain.registry.export.CheckSnapshotServlet.SNAPSHOT_NAME_PARAM; +import static com.google.domain.registry.testing.TaskQueueHelper.assertNoTasksEnqueued; import static com.google.domain.registry.testing.TaskQueueHelper.assertTasksEnqueued; import static javax.servlet.http.HttpServletResponse.SC_ACCEPTED; import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; @@ -22,7 +25,6 @@ import static javax.servlet.http.HttpServletResponse.SC_NOT_MODIFIED; import static javax.servlet.http.HttpServletResponse.SC_OK; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import com.google.common.base.Joiner; @@ -76,9 +78,6 @@ public class CheckSnapshotServletTest { @Mock private DatastoreBackupService backupService; - @Mock - private LoadSnapshotServlet loadSnapshotServlet; - private final FakeClock clock = new FakeClock(COMPLETE_TIME.plusMillis(1000)); private final StringWriter httpOutput = new StringWriter(); private final CheckSnapshotServlet servlet = new CheckSnapshotServlet(); @@ -86,7 +85,6 @@ public class CheckSnapshotServletTest { @Before public void before() throws Exception { inject.setStaticField(CheckSnapshotServlet.class, "backupService", backupService); - inject.setStaticField(CheckSnapshotServlet.class, "loadSnapshotServlet", loadSnapshotServlet); inject.setStaticField(DatastoreBackupInfo.class, "clock", clock); when(rsp.getWriter()).thenReturn(new PrintWriter(httpOutput)); @@ -111,21 +109,34 @@ public class CheckSnapshotServletTest { backupInfo.getGcsFilename()); } + private static void assertLoadTaskEnqueued(String id, String file, String kinds) + throws Exception { + assertTasksEnqueued( + "export-snapshot", + new TaskMatcher() + .url("/_dr/task/loadSnapshot") + .method("POST") + .param("id", id) + .param("file", file) + .param("kinds", kinds)); + } + @Test public void testSuccess_enqueuePollTask() throws Exception { servlet.enqueuePollTask("some_snapshot_name", ImmutableSet.of("one", "two", "three")); assertTasksEnqueued(CheckSnapshotServlet.QUEUE, new TaskMatcher() .url(CheckSnapshotServlet.PATH) - .param(CheckSnapshotServlet.SNAPSHOT_NAME_PARAM, "some_snapshot_name") - .param(CheckSnapshotServlet.SNAPSHOT_KINDS_TO_LOAD_PARAM, "one,two,three") + .param(SNAPSHOT_NAME_PARAM, "some_snapshot_name") + .param(SNAPSHOT_KINDS_TO_LOAD_PARAM, "one,two,three") .method("POST")); } @Test public void testPost_forPendingBackup_returnsNotModified() throws Exception { setPendingBackup(); - when(req.getParameter(CheckSnapshotServlet.SNAPSHOT_NAME_PARAM)).thenReturn("some_backup"); + when(req.getParameter(SNAPSHOT_NAME_PARAM)).thenReturn("some_backup"); + when(req.getParameter(SNAPSHOT_KINDS_TO_LOAD_PARAM)).thenReturn("one,two"); when(backupService.findByName("some_backup")).thenReturn(backupInfo); servlet.service(req, rsp); @@ -135,7 +146,8 @@ public class CheckSnapshotServletTest { @Test public void testPost_forStalePendingBackupBackup_returnsAccepted() throws Exception { setPendingBackup(); - when(req.getParameter(CheckSnapshotServlet.SNAPSHOT_NAME_PARAM)).thenReturn("some_backup"); + when(req.getParameter(SNAPSHOT_NAME_PARAM)).thenReturn("some_backup"); + when(req.getParameter(SNAPSHOT_KINDS_TO_LOAD_PARAM)).thenReturn("one,two"); when(backupService.findByName("some_backup")).thenReturn(backupInfo); clock.setTo(START_TIME .plus(Duration.standardHours(20)) @@ -150,76 +162,57 @@ public class CheckSnapshotServletTest { @Test public void testPost_forCompleteBackup_enqueuesLoadTask() throws Exception { - when(req.getParameter(CheckSnapshotServlet.SNAPSHOT_NAME_PARAM)).thenReturn("some_backup"); - when(req.getParameter(CheckSnapshotServlet.SNAPSHOT_KINDS_TO_LOAD_PARAM)).thenReturn("one,two"); + when(req.getParameter(SNAPSHOT_NAME_PARAM)).thenReturn("some_backup"); + when(req.getParameter(SNAPSHOT_KINDS_TO_LOAD_PARAM)).thenReturn("one,two"); when(backupService.findByName("some_backup")).thenReturn(backupInfo); servlet.service(req, rsp); verify(rsp).setStatus(SC_OK); - verify(loadSnapshotServlet).enqueueLoadTask( - "20140801_010203", - "gs://somebucket/some_backup_20140801.backup_info", - ImmutableSet.of("one", "two")); + assertLoadTaskEnqueued( + "20140801_010203", "gs://somebucket/some_backup_20140801.backup_info", "one,two"); } @Test public void testPost_forCompleteAutoBackup_enqueuesLoadTask_usingBackupName() throws Exception { - when(req.getParameter(CheckSnapshotServlet.SNAPSHOT_NAME_PARAM)) - .thenReturn("auto_snapshot_somestring"); - when(req.getParameter(CheckSnapshotServlet.SNAPSHOT_KINDS_TO_LOAD_PARAM)).thenReturn("one,two"); + when(req.getParameter(SNAPSHOT_NAME_PARAM)).thenReturn("auto_snapshot_somestring"); + when(req.getParameter(SNAPSHOT_KINDS_TO_LOAD_PARAM)).thenReturn("one,two"); when(backupService.findByName("auto_snapshot_somestring")).thenReturn(backupInfo); servlet.service(req, rsp); verify(rsp).setStatus(SC_OK); - verify(loadSnapshotServlet).enqueueLoadTask( - "somestring", - "gs://somebucket/some_backup_20140801.backup_info", - ImmutableSet.of("one", "two")); - } - - @Test - public void testPost_forCompleteBackup_missingKindsToLoad_enqueuesLoadTask() throws Exception { - when(req.getParameter(CheckSnapshotServlet.SNAPSHOT_NAME_PARAM)).thenReturn("some_backup"); - when(backupService.findByName("some_backup")).thenReturn(backupInfo); - - servlet.service(req, rsp); - verify(rsp).setStatus(SC_OK); - verify(loadSnapshotServlet).enqueueLoadTask( - "20140801_010203", - "gs://somebucket/some_backup_20140801.backup_info", - ImmutableSet.of("one", "two", "three")); + assertLoadTaskEnqueued( + "somestring", "gs://somebucket/some_backup_20140801.backup_info", "one,two"); } @Test public void testPost_forCompleteBackup_withExtraKindsToLoad_enqueuesLoadTask() throws Exception { - when(req.getParameter(CheckSnapshotServlet.SNAPSHOT_NAME_PARAM)).thenReturn("some_backup"); - when(req.getParameter(CheckSnapshotServlet.SNAPSHOT_KINDS_TO_LOAD_PARAM)).thenReturn("one,foo"); + when(req.getParameter(SNAPSHOT_NAME_PARAM)).thenReturn("some_backup"); + when(req.getParameter(SNAPSHOT_KINDS_TO_LOAD_PARAM)).thenReturn("one,foo"); when(backupService.findByName("some_backup")).thenReturn(backupInfo); servlet.service(req, rsp); verify(rsp).setStatus(SC_OK); - verify(loadSnapshotServlet).enqueueLoadTask( - "20140801_010203", - "gs://somebucket/some_backup_20140801.backup_info", - ImmutableSet.of("one")); + assertLoadTaskEnqueued( + "20140801_010203", "gs://somebucket/some_backup_20140801.backup_info", "one"); } @Test public void testPost_forCompleteBackup_withEmptyKindsToLoad_skipsLoadTask() throws Exception { - when(req.getParameter(CheckSnapshotServlet.SNAPSHOT_NAME_PARAM)).thenReturn("some_backup"); - when(req.getParameter(CheckSnapshotServlet.SNAPSHOT_KINDS_TO_LOAD_PARAM)).thenReturn(""); + when(req.getParameter(SNAPSHOT_NAME_PARAM)).thenReturn("some_backup"); + when(req.getParameter(SNAPSHOT_KINDS_TO_LOAD_PARAM)).thenReturn(""); when(backupService.findByName("some_backup")).thenReturn(backupInfo); servlet.service(req, rsp); verify(rsp).setStatus(SC_OK); - verifyZeroInteractions(loadSnapshotServlet); + assertNoTasksEnqueued("export-snapshot"); } @Test public void testPost_forBadBackup_returnsBadRequest() throws Exception { - when(req.getParameter(CheckSnapshotServlet.SNAPSHOT_NAME_PARAM)).thenReturn("some_backup"); - when(backupService.findByName("some_backup")).thenThrow( - new IllegalArgumentException("No backup found")); + when(req.getParameter(SNAPSHOT_NAME_PARAM)).thenReturn("some_backup"); + when(req.getParameter(SNAPSHOT_KINDS_TO_LOAD_PARAM)).thenReturn("one,two"); + when(backupService.findByName("some_backup")) + .thenThrow(new IllegalArgumentException("No backup found")); servlet.service(req, rsp); verify(rsp).sendError(SC_BAD_REQUEST, "Bad backup name some_backup: No backup found"); @@ -228,14 +221,16 @@ public class CheckSnapshotServletTest { @Test public void testPost_noBackupSpecified_returnsError() throws Exception { when(req.getMethod()).thenReturn("POST"); + when(req.getParameter(SNAPSHOT_NAME_PARAM)).thenReturn(null); + when(req.getParameter(SNAPSHOT_KINDS_TO_LOAD_PARAM)).thenReturn("one,two"); servlet.service(req, rsp); - verify(rsp).sendError(SC_BAD_REQUEST, "Missing required parameter: name"); + verify(rsp).sendError(SC_BAD_REQUEST, "Missing parameter: name"); } @Test public void testGet_returnsInformation() throws Exception { when(req.getMethod()).thenReturn("GET"); - when(req.getParameter(CheckSnapshotServlet.SNAPSHOT_NAME_PARAM)).thenReturn("some_backup"); + when(req.getParameter(SNAPSHOT_NAME_PARAM)).thenReturn("some_backup"); when(backupService.findByName("some_backup")).thenReturn(backupInfo); servlet.service(req, rsp); @@ -254,7 +249,7 @@ public class CheckSnapshotServletTest { @Test public void testGet_forBadBackup_returnsError() throws Exception { when(req.getMethod()).thenReturn("GET"); - when(req.getParameter(CheckSnapshotServlet.SNAPSHOT_NAME_PARAM)).thenReturn("some_backup"); + when(req.getParameter(SNAPSHOT_NAME_PARAM)).thenReturn("some_backup"); when(backupService.findByName("some_backup")).thenThrow( new IllegalArgumentException("No backup found")); @@ -266,6 +261,6 @@ public class CheckSnapshotServletTest { public void testGet_noBackupSpecified_returnsError() throws Exception { when(req.getMethod()).thenReturn("GET"); servlet.service(req, rsp); - verify(rsp).sendError(SC_BAD_REQUEST, "Missing required parameter: name"); + verify(rsp).sendError(SC_BAD_REQUEST, "Missing parameter: name"); } } diff --git a/javatests/com/google/domain/registry/export/LoadSnapshotServletTest.java b/javatests/com/google/domain/registry/export/LoadSnapshotActionTest.java similarity index 54% rename from javatests/com/google/domain/registry/export/LoadSnapshotServletTest.java rename to javatests/com/google/domain/registry/export/LoadSnapshotActionTest.java index f5721597e..f6c3c218d 100644 --- a/javatests/com/google/domain/registry/export/LoadSnapshotServletTest.java +++ b/javatests/com/google/domain/registry/export/LoadSnapshotActionTest.java @@ -16,21 +16,20 @@ package com.google.domain.registry.export; import static com.google.common.collect.Iterables.transform; import static com.google.common.truth.Truth.assertThat; +import static com.google.domain.registry.export.LoadSnapshotAction.LOAD_SNAPSHOT_FILE_PARAM; +import static com.google.domain.registry.export.LoadSnapshotAction.LOAD_SNAPSHOT_ID_PARAM; +import static com.google.domain.registry.export.LoadSnapshotAction.LOAD_SNAPSHOT_KINDS_PARAM; +import static com.google.domain.registry.export.LoadSnapshotAction.PATH; +import static com.google.domain.registry.export.LoadSnapshotAction.QUEUE; +import static com.google.domain.registry.export.LoadSnapshotAction.enqueueLoadSnapshotTask; import static com.google.domain.registry.testing.TaskQueueHelper.assertTasksEnqueued; -import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; -import static javax.servlet.http.HttpServletResponse.SC_OK; import static org.joda.time.DateTimeZone.UTC; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import com.google.api.client.http.HttpRequestInitializer; -import com.google.api.client.http.HttpTransport; -import com.google.api.client.json.JsonFactory; import com.google.api.services.bigquery.Bigquery; import com.google.api.services.bigquery.model.Dataset; import com.google.api.services.bigquery.model.Job; @@ -41,12 +40,12 @@ import com.google.common.base.Function; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.domain.registry.bigquery.BigqueryFactory; -import com.google.domain.registry.config.TestRegistryConfig; import com.google.domain.registry.export.BigqueryPollJobAction.BigqueryPollJobEnqueuer; +import com.google.domain.registry.request.HttpException.BadRequestException; +import com.google.domain.registry.request.HttpException.InternalServerErrorException; import com.google.domain.registry.testing.AppEngineRule; +import com.google.domain.registry.testing.ExceptionRule; import com.google.domain.registry.testing.FakeClock; -import com.google.domain.registry.testing.InjectRule; -import com.google.domain.registry.testing.RegistryConfigRule; import com.google.domain.registry.testing.TaskQueueHelper.TaskMatcher; import org.joda.time.DateTime; @@ -58,20 +57,12 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; -import java.io.PrintWriter; -import java.io.StringWriter; +import java.io.IOException; import java.util.List; -import javax.servlet.ServletConfig; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -/** Unit tests for {@link LoadSnapshotServlet}. */ +/** Unit tests for {@link LoadSnapshotAction}. */ @RunWith(MockitoJUnitRunner.class) -public class LoadSnapshotServletTest { - - @Rule - public final InjectRule inject = new InjectRule(); +public class LoadSnapshotActionTest { @Rule public final AppEngineRule appEngine = AppEngineRule.builder() @@ -79,21 +70,7 @@ public class LoadSnapshotServletTest { .build(); @Rule - public final RegistryConfigRule configRule = new RegistryConfigRule(new TestRegistryConfig() { - @Override public String getProjectId() { - return "Project-Id"; - } - - @Override public String getSnapshotsDataset() { - return "testdataset"; - } - }); - - @Mock - private HttpServletRequest req; - - @Mock - private HttpServletResponse rsp; + public final ExceptionRule thrown = new ExceptionRule(); @Mock private BigqueryFactory bigqueryFactory; @@ -116,70 +93,48 @@ public class LoadSnapshotServletTest { @Mock private BigqueryPollJobEnqueuer bigqueryPollEnqueuer; - private static final DateTime NOW = new DateTime(1391096117045L, UTC); - - private FakeClock clock = new FakeClock(); - private final StringWriter httpOutput = new StringWriter(); - private final LoadSnapshotServlet servlet = new LoadSnapshotServlet(); + private FakeClock clock = new FakeClock(new DateTime(1391096117045L, UTC)); + private LoadSnapshotAction action; @Before public void before() throws Exception { - clock.setTo(NOW); - inject.setStaticField(LoadSnapshotServlet.class, "clock", clock); - inject.setStaticField(LoadSnapshotServlet.class, "bigqueryFactory", bigqueryFactory); - inject.setStaticField(LoadSnapshotServlet.class, "bigqueryPollEnqueuer", bigqueryPollEnqueuer); - - when(rsp.getWriter()).thenReturn(new PrintWriter(httpOutput)); - - when(bigqueryFactory.create( - anyString(), - any(HttpTransport.class), - any(JsonFactory.class), - any(HttpRequestInitializer.class))) - .thenReturn(bigquery); - + when(bigqueryFactory.create("Project-Id", "snapshots")).thenReturn(bigquery); when(bigquery.jobs()).thenReturn(bigqueryJobs); when(bigqueryJobs.insert(eq("Project-Id"), any(Job.class))).thenReturn(bigqueryJobsInsert); - when(bigquery.datasets()).thenReturn(bigqueryDatasets); when(bigqueryDatasets.insert(eq("Project-Id"), any(Dataset.class))) .thenReturn(bigqueryDatasetsInsert); - - servlet.init(mock(ServletConfig.class)); - when(req.getMethod()).thenReturn("POST"); + action = new LoadSnapshotAction(); + action.bigqueryFactory = bigqueryFactory; + action.bigqueryPollEnqueuer = bigqueryPollEnqueuer; + action.clock = clock; + action.projectId = "Project-Id"; + action.snapshotFile = "gs://bucket/snapshot.backup_info"; + action.snapshotId = "id12345"; + action.snapshotKinds = "one,two,three"; } @Test public void testSuccess_enqueueLoadTask() throws Exception { - servlet.enqueueLoadTask( + enqueueLoadSnapshotTask( "id12345", "gs://bucket/snapshot.backup_info", ImmutableSet.of("one", "two", "three")); - assertTasksEnqueued(LoadSnapshotServlet.QUEUE, + assertTasksEnqueued( + QUEUE, new TaskMatcher() - .url(LoadSnapshotServlet.PATH) + .url(PATH) .method("POST") - .param(LoadSnapshotServlet.SNAPSHOT_ID_PARAM, "id12345") - .param(LoadSnapshotServlet.SNAPSHOT_FILE_PARAM, "gs://bucket/snapshot.backup_info") - .param(LoadSnapshotServlet.SNAPSHOT_KINDS_PARAM, "one,two,three")); + .param(LOAD_SNAPSHOT_ID_PARAM, "id12345") + .param(LOAD_SNAPSHOT_FILE_PARAM, "gs://bucket/snapshot.backup_info") + .param(LOAD_SNAPSHOT_KINDS_PARAM, "one,two,three")); } @Test public void testSuccess_doPost() throws Exception { - when(req.getParameter(LoadSnapshotServlet.SNAPSHOT_ID_PARAM)).thenReturn("id12345"); - when(req.getParameter(LoadSnapshotServlet.SNAPSHOT_FILE_PARAM)) - .thenReturn("gs://bucket/snapshot.backup_info"); - when(req.getParameter(LoadSnapshotServlet.SNAPSHOT_KINDS_PARAM)) - .thenReturn("one,two,three"); + action.run(); - servlet.service(req, rsp); - - // Check that we attempted to create the snapshots dataset. - ArgumentCaptor datasetArgument = ArgumentCaptor.forClass(Dataset.class); - verify(bigqueryDatasets).insert(eq("Project-Id"), datasetArgument.capture()); - assertThat(datasetArgument.getValue().getDatasetReference().getProjectId()) - .isEqualTo("Project-Id"); - assertThat(datasetArgument.getValue().getDatasetReference().getDatasetId()) - .isEqualTo("testdataset"); - verify(bigqueryDatasetsInsert).execute(); + // Verify that bigqueryFactory was called in a way that would create the dataset if it didn't + // already exist. + verify(bigqueryFactory).create("Project-Id", "snapshots"); // Capture the load jobs we inserted to do additional checking on them. ArgumentCaptor jobArgument = ArgumentCaptor.forClass(Job.class); @@ -193,7 +148,7 @@ public class LoadSnapshotServletTest { JobConfigurationLoad config = job.getConfiguration().getLoad(); assertThat(config.getSourceFormat()).isEqualTo("DATASTORE_BACKUP"); assertThat(config.getDestinationTable().getProjectId()).isEqualTo("Project-Id"); - assertThat(config.getDestinationTable().getDatasetId()).isEqualTo("testdataset"); + assertThat(config.getDestinationTable().getDatasetId()).isEqualTo("snapshots"); } // Check the job IDs for each load job. @@ -231,64 +186,37 @@ public class LoadSnapshotServletTest { new JobReference() .setProjectId("Project-Id") .setJobId("load-snapshot-id12345-one-1391096117045"), - UpdateSnapshotViewAction.createViewUpdateTask("testdataset", "id12345_one", "one"), + UpdateSnapshotViewAction.createViewUpdateTask("snapshots", "id12345_one", "one"), QueueFactory.getQueue(UpdateSnapshotViewAction.QUEUE)); verify(bigqueryPollEnqueuer).enqueuePollTask( new JobReference() .setProjectId("Project-Id") .setJobId("load-snapshot-id12345-two-1391096117045"), - UpdateSnapshotViewAction.createViewUpdateTask("testdataset", "id12345_two", "two"), + UpdateSnapshotViewAction.createViewUpdateTask("snapshots", "id12345_two", "two"), QueueFactory.getQueue(UpdateSnapshotViewAction.QUEUE)); verify(bigqueryPollEnqueuer).enqueuePollTask( new JobReference() .setProjectId("Project-Id") .setJobId("load-snapshot-id12345-three-1391096117045"), - UpdateSnapshotViewAction.createViewUpdateTask("testdataset", "id12345_three", "three"), + UpdateSnapshotViewAction.createViewUpdateTask("snapshots", "id12345_three", "three"), QueueFactory.getQueue(UpdateSnapshotViewAction.QUEUE)); - - verify(rsp).setStatus(SC_OK); - } - - @Test - public void testFailure_doPost_missingIdHeader() throws Exception { - when(req.getParameter(LoadSnapshotServlet.SNAPSHOT_FILE_PARAM)) - .thenReturn("gs://bucket/snapshot.backup_info"); - when(req.getParameter(LoadSnapshotServlet.SNAPSHOT_KINDS_PARAM)) - .thenReturn("one,two,three"); - - servlet.service(req, rsp); - verify(rsp).sendError(SC_BAD_REQUEST, "Missing required parameter: id"); - } - - @Test - public void testFailure_doPost_missingFileHeader() throws Exception { - when(req.getParameter(LoadSnapshotServlet.SNAPSHOT_ID_PARAM)).thenReturn("id12345"); - when(req.getParameter(LoadSnapshotServlet.SNAPSHOT_KINDS_PARAM)) - .thenReturn("one,two,three"); - - servlet.service(req, rsp); - verify(rsp).sendError(SC_BAD_REQUEST, "Missing required parameter: file"); - } - - @Test - public void testFailure_doPost_missingKindHeader() throws Exception { - when(req.getParameter(LoadSnapshotServlet.SNAPSHOT_ID_PARAM)).thenReturn("id12345"); - when(req.getParameter(LoadSnapshotServlet.SNAPSHOT_FILE_PARAM)) - .thenReturn("gs://bucket/snapshot.backup_info"); - - servlet.service(req, rsp); - verify(rsp).sendError(SC_BAD_REQUEST, "Missing required parameter: kinds"); } @Test public void testFailure_doPost_badGcsFilename() throws Exception { - when(req.getParameter(LoadSnapshotServlet.SNAPSHOT_ID_PARAM)).thenReturn("id12345"); - when(req.getParameter(LoadSnapshotServlet.SNAPSHOT_FILE_PARAM)) - .thenReturn("gs://bucket/snapshot"); - when(req.getParameter(LoadSnapshotServlet.SNAPSHOT_KINDS_PARAM)) - .thenReturn("one,two,three"); + action.snapshotFile = "gs://bucket/snapshot"; + thrown.expect( + BadRequestException.class, + "Error calling load snapshot: backup info file extension missing"); + action.run(); + } - servlet.service(req, rsp); - verify(rsp).sendError(SC_BAD_REQUEST, "backup info file extension missing"); + @Test + public void testFailure_doPost_bigqueryThrowsException() throws Exception { + when(bigqueryJobsInsert.execute()).thenThrow(new IOException("The Internet has gone missing")); + thrown.expect( + InternalServerErrorException.class, + "Error loading snapshot: The Internet has gone missing"); + action.run(); } } diff --git a/javatests/com/google/domain/registry/export/UpdateSnapshotViewActionTest.java b/javatests/com/google/domain/registry/export/UpdateSnapshotViewActionTest.java index 14c63c225..457818e91 100644 --- a/javatests/com/google/domain/registry/export/UpdateSnapshotViewActionTest.java +++ b/javatests/com/google/domain/registry/export/UpdateSnapshotViewActionTest.java @@ -15,9 +15,9 @@ package com.google.domain.registry.export; import static com.google.common.truth.Truth.assertThat; -import static com.google.domain.registry.export.UpdateSnapshotViewAction.SNAPSHOT_DATASET_ID_PARAM; -import static com.google.domain.registry.export.UpdateSnapshotViewAction.SNAPSHOT_KIND_PARAM; -import static com.google.domain.registry.export.UpdateSnapshotViewAction.SNAPSHOT_TABLE_ID_PARAM; +import static com.google.domain.registry.export.UpdateSnapshotViewAction.UPDATE_SNAPSHOT_DATASET_ID_PARAM; +import static com.google.domain.registry.export.UpdateSnapshotViewAction.UPDATE_SNAPSHOT_KIND_PARAM; +import static com.google.domain.registry.export.UpdateSnapshotViewAction.UPDATE_SNAPSHOT_TABLE_ID_PARAM; import static com.google.domain.registry.testing.TaskQueueHelper.assertTasksEnqueued; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; @@ -30,11 +30,9 @@ import com.google.api.services.bigquery.model.Dataset; import com.google.api.services.bigquery.model.Table; import com.google.appengine.api.taskqueue.QueueFactory; import com.google.domain.registry.bigquery.BigqueryFactory; -import com.google.domain.registry.config.TestRegistryConfig; import com.google.domain.registry.request.HttpException.InternalServerErrorException; import com.google.domain.registry.testing.AppEngineRule; import com.google.domain.registry.testing.ExceptionRule; -import com.google.domain.registry.testing.RegistryConfigRule; import com.google.domain.registry.testing.TaskQueueHelper.TaskMatcher; import org.junit.Before; @@ -59,17 +57,6 @@ public class UpdateSnapshotViewActionTest { @Rule public final ExceptionRule thrown = new ExceptionRule(); - @Rule - public final RegistryConfigRule configRule = new RegistryConfigRule(new TestRegistryConfig() { - @Override public String getProjectId() { - return "Project-Id"; - } - - @Override public String getLatestSnapshotDataset() { - return "testdataset"; - } - }); - @Mock private BigqueryFactory bigqueryFactory; @@ -104,8 +91,9 @@ public class UpdateSnapshotViewActionTest { action = new UpdateSnapshotViewAction(); action.bigqueryFactory = bigqueryFactory; action.datasetId = "some_dataset"; - action.tableId = "12345_fookind"; action.kindName = "fookind"; + action.projectId = "Project-Id"; + action.tableId = "12345_fookind"; } @Test @@ -116,9 +104,9 @@ public class UpdateSnapshotViewActionTest { new TaskMatcher() .url(UpdateSnapshotViewAction.PATH) .method("POST") - .param(SNAPSHOT_DATASET_ID_PARAM, "some_dataset") - .param(SNAPSHOT_TABLE_ID_PARAM, "12345_fookind") - .param(SNAPSHOT_KIND_PARAM, "fookind")); + .param(UPDATE_SNAPSHOT_DATASET_ID_PARAM, "some_dataset") + .param(UPDATE_SNAPSHOT_TABLE_ID_PARAM, "12345_fookind") + .param(UPDATE_SNAPSHOT_KIND_PARAM, "fookind")); } @Test @@ -128,7 +116,7 @@ public class UpdateSnapshotViewActionTest { // Check that we updated the view. ArgumentCaptor tableArg = ArgumentCaptor.forClass(Table.class); verify(bigqueryTables).update( - eq("Project-Id"), eq("testdataset"), eq("fookind"), tableArg.capture()); + eq("Project-Id"), eq("latest_snapshot"), eq("fookind"), tableArg.capture()); assertThat(tableArg.getValue().getView().getQuery()) .isEqualTo("SELECT * FROM [some_dataset.12345_fookind]"); }