From 7ee541e1b1faec2ab30c9e774f874a54240485cf Mon Sep 17 00:00:00 2001 From: Juan Celhay Date: Tue, 17 Sep 2024 12:02:25 -0400 Subject: [PATCH 1/4] Add activeOrDeletedSince parameter to RefreshDnsForAllDomainsAction (#2556) --- .../server/RefreshDnsForAllDomainsAction.java | 19 ++++-- .../tools/server/ToolsServerModule.java | 8 +++ .../RefreshDnsForAllDomainsActionTest.java | 36 ++++++++++- .../tools/server/ToolsServerModuleTest.java | 59 +++++++++++++++++++ 4 files changed, 115 insertions(+), 7 deletions(-) create mode 100644 core/src/test/java/google/registry/tools/server/ToolsServerModuleTest.java diff --git a/core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java b/core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java index 9d64d7caabc..a4f876e39f6 100644 --- a/core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java +++ b/core/src/main/java/google/registry/tools/server/RefreshDnsForAllDomainsAction.java @@ -39,6 +39,7 @@ import javax.inject.Inject; import org.apache.arrow.util.VisibleForTesting; import org.apache.http.HttpStatus; +import org.joda.time.DateTime; import org.joda.time.Duration; /** @@ -50,6 +51,10 @@ * *

You may pass in a {@code batchSize} for the batched read of domains from the database. This is * recommended to be somewhere between 200 and 500. The default value is 250. + * + *

If {@code activeOrDeletedSince} is passed in the request, this action will enqueue DNS publish + * tasks on all domains with a deletion time equal or greater than the value provided, including + * domains that have since been deleted. */ @Action( service = Action.Service.TOOLS, @@ -80,17 +85,21 @@ public class RefreshDnsForAllDomainsAction implements Runnable { private final Random random; + private final DateTime activeOrDeletedSince; + @Inject RefreshDnsForAllDomainsAction( Response response, @Parameter(PARAM_TLDS) ImmutableSet tlds, @Parameter(PARAM_BATCH_SIZE) Optional batchSize, @Parameter("refreshQps") Optional refreshQps, + @Parameter("activeOrDeletedSince") Optional activeOrDeletedSince, Random random) { this.response = response; this.tlds = tlds; this.batchSize = batchSize.orElse(DEFAULT_BATCH_SIZE); this.refreshQps = refreshQps.orElse(DEFAULT_REFRESH_QPS); + this.activeOrDeletedSince = activeOrDeletedSince.orElse(END_OF_TIME); this.random = random; } @@ -118,11 +127,11 @@ public void run() { private Duration calculateSmear() { Long activeDomains = tm().query( - "SELECT COUNT(*) FROM Domain WHERE tld IN (:tlds) AND deletionTime =" - + " :endOfTime", + "SELECT COUNT(*) FROM Domain WHERE tld IN (:tlds) AND deletionTime >=" + + " :activeOrDeletedSince", Long.class) .setParameter("tlds", tlds) - .setParameter("endOfTime", END_OF_TIME) + .setParameter("activeOrDeletedSince", activeOrDeletedSince) .getSingleResult(); return Duration.standardSeconds(Math.max(activeDomains / refreshQps, 1)); } @@ -131,12 +140,12 @@ private ImmutableList getBatch(Optional lastInPreviousBatch) { String sql = String.format( "SELECT domainName FROM Domain WHERE tld IN (:tlds) AND" - + " deletionTime = :endOfTime %s ORDER BY domainName ASC", + + " deletionTime >= :activeOrDeletedSince %s ORDER BY domainName ASC", lastInPreviousBatch.isPresent() ? "AND domainName > :lastInPreviousBatch" : ""); TypedQuery query = tm().query(sql, String.class) .setParameter("tlds", tlds) - .setParameter("endOfTime", END_OF_TIME); + .setParameter("activeOrDeletedSince", activeOrDeletedSince); lastInPreviousBatch.ifPresent(l -> query.setParameter("lastInPreviousBatch", l)); return query.setMaxResults(batchSize).getResultStream().collect(toImmutableList()); } diff --git a/core/src/main/java/google/registry/tools/server/ToolsServerModule.java b/core/src/main/java/google/registry/tools/server/ToolsServerModule.java index c7979b84875..36665215641 100644 --- a/core/src/main/java/google/registry/tools/server/ToolsServerModule.java +++ b/core/src/main/java/google/registry/tools/server/ToolsServerModule.java @@ -16,6 +16,7 @@ import static com.google.common.base.Strings.emptyToNull; import static google.registry.request.RequestParameters.extractIntParameter; +import static google.registry.request.RequestParameters.extractOptionalDatetimeParameter; import static google.registry.request.RequestParameters.extractOptionalIntParameter; import static google.registry.request.RequestParameters.extractOptionalParameter; import static google.registry.request.RequestParameters.extractRequiredParameter; @@ -26,6 +27,7 @@ import google.registry.tools.server.UpdateUserGroupAction.Mode; import jakarta.servlet.http.HttpServletRequest; import java.util.Optional; +import org.joda.time.DateTime; /** Dagger module for the tools package. */ @Module @@ -75,6 +77,12 @@ static Optional provideRefreshQps(HttpServletRequest req) { return extractOptionalIntParameter(req, "refreshQps"); } + @Provides + @Parameter("activeOrDeletedSince") + static Optional provideDeletionTime(HttpServletRequest req) { + return extractOptionalDatetimeParameter(req, "activeOrDeletedSince"); + } + @Provides static Mode provideGroupUpdateMode(HttpServletRequest req) { return Mode.valueOf(extractRequiredParameter(req, "groupUpdateMode")); diff --git a/core/src/test/java/google/registry/tools/server/RefreshDnsForAllDomainsActionTest.java b/core/src/test/java/google/registry/tools/server/RefreshDnsForAllDomainsActionTest.java index e1c64389395..07a5800303e 100644 --- a/core/src/test/java/google/registry/tools/server/RefreshDnsForAllDomainsActionTest.java +++ b/core/src/test/java/google/registry/tools/server/RefreshDnsForAllDomainsActionTest.java @@ -56,7 +56,12 @@ void beforeEach() { createTld("bar"); action = new RefreshDnsForAllDomainsAction( - response, ImmutableSet.of("bar"), Optional.of(10), Optional.empty(), new Random()); + response, + ImmutableSet.of("bar"), + Optional.of(10), + Optional.empty(), + Optional.empty(), + new Random()); } @Test @@ -75,7 +80,12 @@ void test_runAction_smearsOutDnsRefreshes() throws Exception { // Set batch size to 1 since each batch will be enqueud at the same time action = new RefreshDnsForAllDomainsAction( - response, ImmutableSet.of("bar"), Optional.of(1), Optional.of(7), new Random()); + response, + ImmutableSet.of("bar"), + Optional.of(1), + Optional.of(7), + Optional.empty(), + new Random()); tm().transact(() -> action.refreshBatch(Optional.empty(), Duration.standardMinutes(1000))); tm().transact(() -> action.refreshBatch(Optional.empty(), Duration.standardMinutes(1000))); ImmutableList refreshRequests = @@ -98,6 +108,28 @@ void test_runAction_doesntRefreshDeletedDomain() throws Exception { assertNoDnsRequestsExcept("foo.bar"); } + @Test + void test_runAction_refreshesDeletedDomain_whenActiveOrDeletedSinceIsProvided() throws Exception { + action = + new RefreshDnsForAllDomainsAction( + response, + ImmutableSet.of("bar"), + Optional.of(1), + Optional.of(7), + Optional.of(clock.nowUtc().minusYears(3)), + new Random()); + persistActiveDomain("foo.bar"); + persistDeletedDomain("deleted1.bar", clock.nowUtc().minusYears(1)); + persistDeletedDomain("deleted3.bar", clock.nowUtc().minusYears(3)); + persistDeletedDomain("deleted5.bar", clock.nowUtc().minusYears(5)); + action.run(); + assertDomainDnsRequestWithRequestTime("foo.bar", clock.nowUtc()); + assertDomainDnsRequestWithRequestTime("deleted1.bar", clock.nowUtc()); + assertDomainDnsRequestWithRequestTime("deleted3.bar", clock.nowUtc()); + + assertNoDnsRequestsExcept("foo.bar", "deleted1.bar", "deleted3.bar"); + } + @Test void test_runAction_ignoresDomainsOnOtherTlds() throws Exception { createTld("baz"); diff --git a/core/src/test/java/google/registry/tools/server/ToolsServerModuleTest.java b/core/src/test/java/google/registry/tools/server/ToolsServerModuleTest.java new file mode 100644 index 00000000000..a87485ee30c --- /dev/null +++ b/core/src/test/java/google/registry/tools/server/ToolsServerModuleTest.java @@ -0,0 +1,59 @@ +// Copyright 2017 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.tools.server; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import google.registry.request.HttpException.BadRequestException; +import jakarta.servlet.http.HttpServletRequest; +import java.util.Optional; +import org.joda.time.DateTime; +import org.junit.jupiter.api.Test; + +/** Unit tests for {@link ToolsServerModule}. */ +public class ToolsServerModuleTest { + + private final HttpServletRequest request = mock(HttpServletRequest.class); + + @Test + void test_provideDeletionTime() throws Exception { + when(request.getParameter("activeOrDeletedSince")).thenReturn("1991-07-01T00:00:00Z"); + + DateTime expected = DateTime.parse("1991-07-01T00:00:00Z"); + Optional dateTimeParam = ToolsServerModule.provideDeletionTime(request); + + assertThat(dateTimeParam).isEqualTo(Optional.of(expected)); + } + + @Test + void test_doesNotprovideDeletionTimeOnEmptyParam() throws Exception { + when(request.getParameter("activeOrDeletedSince")).thenReturn(""); + + assertThat(ToolsServerModule.provideDeletionTime(request)).isEqualTo(Optional.empty()); + } + + @Test + void test_provideDeletionTime_incorrectDateFormat_throwsBadRequestException() throws Exception { + when(request.getParameter("activeOrDeletedSince")).thenReturn("error404?"); + + BadRequestException thrown = + assertThrows( + BadRequestException.class, () -> ToolsServerModule.provideDeletionTime(request)); + assertThat(thrown).hasMessageThat().contains("Bad ISO 8601 timestamp: activeOrDeletedSince"); + } +} From a988732d651308eb4aa0aebefff23d718379dff8 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 17 Sep 2024 18:40:24 -0400 Subject: [PATCH 2/4] Bump the npm_and_yarn group in /console-webapp with 5 updates (#2560) Bumps the npm_and_yarn group in /console-webapp with 5 updates: | Package | From | To | | --- | --- | --- | | [body-parser](https://github.com/expressjs/body-parser) | `1.20.2` | `1.20.3` | | [express](https://github.com/expressjs/express) | `4.19.2` | `4.21.0` | | [path-to-regexp](https://github.com/pillarjs/path-to-regexp) | `0.1.7` | `0.1.10` | | [send](https://github.com/pillarjs/send) | `0.18.0` | `0.19.0` | | [serve-static](https://github.com/expressjs/serve-static) | `1.15.0` | `1.16.2` | Updates `body-parser` from 1.20.2 to 1.20.3 - [Release notes](https://github.com/expressjs/body-parser/releases) - [Changelog](https://github.com/expressjs/body-parser/blob/master/HISTORY.md) - [Commits](https://github.com/expressjs/body-parser/compare/1.20.2...1.20.3) Updates `express` from 4.19.2 to 4.21.0 - [Release notes](https://github.com/expressjs/express/releases) - [Changelog](https://github.com/expressjs/express/blob/4.21.0/History.md) - [Commits](https://github.com/expressjs/express/compare/4.19.2...4.21.0) Updates `express` from 4.19.2 to 4.21.0 - [Release notes](https://github.com/expressjs/express/releases) - [Changelog](https://github.com/expressjs/express/blob/4.21.0/History.md) - [Commits](https://github.com/expressjs/express/compare/4.19.2...4.21.0) Updates `path-to-regexp` from 0.1.7 to 0.1.10 - [Release notes](https://github.com/pillarjs/path-to-regexp/releases) - [Changelog](https://github.com/pillarjs/path-to-regexp/blob/master/History.md) - [Commits](https://github.com/pillarjs/path-to-regexp/compare/v0.1.7...v0.1.10) Updates `send` from 0.18.0 to 0.19.0 - [Release notes](https://github.com/pillarjs/send/releases) - [Changelog](https://github.com/pillarjs/send/blob/master/HISTORY.md) - [Commits](https://github.com/pillarjs/send/compare/0.18.0...0.19.0) Updates `serve-static` from 1.15.0 to 1.16.2 - [Release notes](https://github.com/expressjs/serve-static/releases) - [Changelog](https://github.com/expressjs/serve-static/blob/v1.16.2/HISTORY.md) - [Commits](https://github.com/expressjs/serve-static/compare/v1.15.0...v1.16.2) --- updated-dependencies: - dependency-name: body-parser dependency-type: indirect dependency-group: npm_and_yarn - dependency-name: express dependency-type: indirect dependency-group: npm_and_yarn - dependency-name: express dependency-type: indirect dependency-group: npm_and_yarn - dependency-name: path-to-regexp dependency-type: indirect dependency-group: npm_and_yarn - dependency-name: send dependency-type: indirect dependency-group: npm_and_yarn - dependency-name: serve-static dependency-type: indirect dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- console-webapp/package-lock.json | 106 +++++++++++++++++++------------ 1 file changed, 65 insertions(+), 41 deletions(-) diff --git a/console-webapp/package-lock.json b/console-webapp/package-lock.json index 0867f0a2cec..4c19e1575eb 100644 --- a/console-webapp/package-lock.json +++ b/console-webapp/package-lock.json @@ -6625,9 +6625,9 @@ } }, "node_modules/body-parser": { - "version": "1.20.2", - "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-1.20.2.tgz", - "integrity": "sha512-ml9pReCu3M61kGlqoTm2umSXTlRTuGTx0bfYj+uIUKKYycG5NtSbeetV3faSU6R7ajOPw0g/J1PvK4qNy7s5bA==", + "version": "1.20.3", + "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-1.20.3.tgz", + "integrity": "sha512-7rAxByjUMqQ3/bHJy7D6OGXvx/MMc4IqBn/X0fcM1QUcAItpZrBEYhWGem+tzXH90c+G01ypMcYJBO9Y30203g==", "dev": true, "dependencies": { "bytes": "3.1.2", @@ -6638,7 +6638,7 @@ "http-errors": "2.0.0", "iconv-lite": "0.4.24", "on-finished": "2.4.1", - "qs": "6.11.0", + "qs": "6.13.0", "raw-body": "2.5.2", "type-is": "~1.6.18", "unpipe": "1.0.0" @@ -8819,37 +8819,37 @@ "dev": true }, "node_modules/express": { - "version": "4.19.2", - "resolved": "https://registry.npmjs.org/express/-/express-4.19.2.tgz", - "integrity": "sha512-5T6nhjsT+EOMzuck8JjBHARTHfMht0POzlA60WV2pMD3gyXw2LZnZ+ueGdNxG+0calOJcWKbpFcuzLZ91YWq9Q==", + "version": "4.21.0", + "resolved": "https://registry.npmjs.org/express/-/express-4.21.0.tgz", + "integrity": "sha512-VqcNGcj/Id5ZT1LZ/cfihi3ttTn+NJmkli2eZADigjq29qTlWi/hAQ43t/VLPq8+UX06FCEx3ByOYet6ZFblng==", "dev": true, "dependencies": { "accepts": "~1.3.8", "array-flatten": "1.1.1", - "body-parser": "1.20.2", + "body-parser": "1.20.3", "content-disposition": "0.5.4", "content-type": "~1.0.4", "cookie": "0.6.0", "cookie-signature": "1.0.6", "debug": "2.6.9", "depd": "2.0.0", - "encodeurl": "~1.0.2", + "encodeurl": "~2.0.0", "escape-html": "~1.0.3", "etag": "~1.8.1", - "finalhandler": "1.2.0", + "finalhandler": "1.3.1", "fresh": "0.5.2", "http-errors": "2.0.0", - "merge-descriptors": "1.0.1", + "merge-descriptors": "1.0.3", "methods": "~1.1.2", "on-finished": "2.4.1", "parseurl": "~1.3.3", - "path-to-regexp": "0.1.7", + "path-to-regexp": "0.1.10", "proxy-addr": "~2.0.7", - "qs": "6.11.0", + "qs": "6.13.0", "range-parser": "~1.2.1", "safe-buffer": "5.2.1", - "send": "0.18.0", - "serve-static": "1.15.0", + "send": "0.19.0", + "serve-static": "1.16.2", "setprototypeof": "1.2.0", "statuses": "2.0.1", "type-is": "~1.6.18", @@ -8878,14 +8878,23 @@ "ms": "2.0.0" } }, + "node_modules/express/node_modules/encodeurl": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/encodeurl/-/encodeurl-2.0.0.tgz", + "integrity": "sha512-Q0n9HRi4m6JuGIV1eFlmvJB7ZEVxu93IrMyiMsGC0lrMJMWzRgx6WGquyfQgZVb31vhGgXnfmPNNXmxnOkRBrg==", + "dev": true, + "engines": { + "node": ">= 0.8" + } + }, "node_modules/express/node_modules/finalhandler": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/finalhandler/-/finalhandler-1.2.0.tgz", - "integrity": "sha512-5uXcUVftlQMFnWC9qu/svkWv3GTd2PfUhK/3PLkYNAe7FbqJMt3515HaxE6eRL74GdsriiwujiawdaB1BpEISg==", + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/finalhandler/-/finalhandler-1.3.1.tgz", + "integrity": "sha512-6BN9trH7bp3qvnrRyzsBz+g3lZxTNZTbVO2EV1CS0WIcDbawYVdYvGflME/9QP0h0pYlCDBCTjYa9nZzMDpyxQ==", "dev": true, "dependencies": { "debug": "2.6.9", - "encodeurl": "~1.0.2", + "encodeurl": "~2.0.0", "escape-html": "~1.0.3", "on-finished": "2.4.1", "parseurl": "~1.3.3", @@ -11420,10 +11429,13 @@ } }, "node_modules/merge-descriptors": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/merge-descriptors/-/merge-descriptors-1.0.1.tgz", - "integrity": "sha512-cCi6g3/Zr1iqQi6ySbseM1Xvooa98N0w31jzUYrXPX2xqObmFGHJ0tQ5u74H3mVh7wLouTseZyYIq39g8cNp1w==", - "dev": true + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/merge-descriptors/-/merge-descriptors-1.0.3.tgz", + "integrity": "sha512-gaNvAS7TZ897/rVaZ0nMtAyxNyi/pdbjbAwUpFQpN70GqnVfOiXpeUUMKRBmzXaSQ8DdTX4/0ms62r2K+hE6mQ==", + "dev": true, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } }, "node_modules/merge-stream": { "version": "2.0.0", @@ -12431,10 +12443,13 @@ } }, "node_modules/object-inspect": { - "version": "1.13.1", - "resolved": "https://registry.npmjs.org/object-inspect/-/object-inspect-1.13.1.tgz", - "integrity": "sha512-5qoj1RUiKOMsCCNLV1CBiPYE10sziTsnmNxkAI/rZhiD63CF7IqdFGC/XzjWjpSgLf0LxXX3bDFIh0E18f6UhQ==", + "version": "1.13.2", + "resolved": "https://registry.npmjs.org/object-inspect/-/object-inspect-1.13.2.tgz", + "integrity": "sha512-IRZSRuzJiynemAXPYtPe5BoI/RESNYR7TYm50MC5Mqbd3Jmw5y790sErYw3V6SryFJD64b74qQQs9wn5Bg/k3g==", "dev": true, + "engines": { + "node": ">= 0.4" + }, "funding": { "url": "https://github.com/sponsors/ljharb" } @@ -12897,9 +12912,9 @@ } }, "node_modules/path-to-regexp": { - "version": "0.1.7", - "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-0.1.7.tgz", - "integrity": "sha512-5DFkuoqlv1uYQKxy8omFBeJPQcdoE07Kv2sferDCrAq1ohOU+MSDswDIbnx3YAM60qIOnYa53wBhXW0EbMonrQ==", + "version": "0.1.10", + "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-0.1.10.tgz", + "integrity": "sha512-7lf7qcQidTku0Gu3YDPc8DJ1q7OOucfa/BSsIwjuh56VU7katFvuM8hULfkwB3Fns/rsVF7PwPKVw1sl5KQS9w==", "dev": true }, "node_modules/path-type": { @@ -13326,12 +13341,12 @@ } }, "node_modules/qs": { - "version": "6.11.0", - "resolved": "https://registry.npmjs.org/qs/-/qs-6.11.0.tgz", - "integrity": "sha512-MvjoMCJwEarSbUYk5O+nmoSzSutSsTwF85zcHPQ9OrlFoZOYIjaqBAJIqIXjptyD5vThxGq52Xu/MaJzRkIk4Q==", + "version": "6.13.0", + "resolved": "https://registry.npmjs.org/qs/-/qs-6.13.0.tgz", + "integrity": "sha512-+38qI9SOr8tfZ4QmJNplMUxqjbe7LKvvZgWdExBOmd+egZTtjLB67Gu0HRX3u/XOq7UU2Nx6nsjvS16Z9uwfpg==", "dev": true, "dependencies": { - "side-channel": "^1.0.4" + "side-channel": "^1.0.6" }, "engines": { "node": ">=0.6" @@ -13916,9 +13931,9 @@ "dev": true }, "node_modules/send": { - "version": "0.18.0", - "resolved": "https://registry.npmjs.org/send/-/send-0.18.0.tgz", - "integrity": "sha512-qqWzuOjSFOuqPjFe4NOsMLafToQQwBSOEpS+FwEt3A2V3vKubTquT3vmLTQpFgMXp8AlFWFuP1qKaJZOtPpVXg==", + "version": "0.19.0", + "resolved": "https://registry.npmjs.org/send/-/send-0.19.0.tgz", + "integrity": "sha512-dW41u5VfLXu8SJh5bwRmyYUbAoSB3c9uQh6L8h/KtsFREPWpbX1lrljJo186Jc4nmci/sGUZ9a0a0J2zgfq2hw==", "dev": true, "dependencies": { "debug": "2.6.9", @@ -14060,20 +14075,29 @@ "dev": true }, "node_modules/serve-static": { - "version": "1.15.0", - "resolved": "https://registry.npmjs.org/serve-static/-/serve-static-1.15.0.tgz", - "integrity": "sha512-XGuRDNjXUijsUL0vl6nSD7cwURuzEgglbOaFuZM9g3kwDXOWVTck0jLzjPzGD+TazWbboZYu52/9/XPdUgne9g==", + "version": "1.16.2", + "resolved": "https://registry.npmjs.org/serve-static/-/serve-static-1.16.2.tgz", + "integrity": "sha512-VqpjJZKadQB/PEbEwvFdO43Ax5dFBZ2UECszz8bQ7pi7wt//PWe1P6MN7eCnjsatYtBT6EuiClbjSWP2WrIoTw==", "dev": true, "dependencies": { - "encodeurl": "~1.0.2", + "encodeurl": "~2.0.0", "escape-html": "~1.0.3", "parseurl": "~1.3.3", - "send": "0.18.0" + "send": "0.19.0" }, "engines": { "node": ">= 0.8.0" } }, + "node_modules/serve-static/node_modules/encodeurl": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/encodeurl/-/encodeurl-2.0.0.tgz", + "integrity": "sha512-Q0n9HRi4m6JuGIV1eFlmvJB7ZEVxu93IrMyiMsGC0lrMJMWzRgx6WGquyfQgZVb31vhGgXnfmPNNXmxnOkRBrg==", + "dev": true, + "engines": { + "node": ">= 0.8" + } + }, "node_modules/set-function-length": { "version": "1.2.2", "resolved": "https://registry.npmjs.org/set-function-length/-/set-function-length-1.2.2.tgz", From febdbc046816c960dd7a9e49e8ddad4a2831731f Mon Sep 17 00:00:00 2001 From: Weimin Yu Date: Wed, 18 Sep 2024 14:18:36 -0400 Subject: [PATCH 3/4] Drop the `transact` call in IdService (#2561) * Drop the `transact` call in Id services All usages already routed through `tm().allocateId()`, which is guaranteed to be in a transaction. * Addressing reviews --- .../persistence/transaction/IdService.java | 42 ------------------- .../JpaTransactionManagerImpl.java | 35 +++++++++++++++- .../transaction/ReplicaDbIdService.java | 38 ----------------- 3 files changed, 34 insertions(+), 81 deletions(-) delete mode 100644 core/src/main/java/google/registry/persistence/transaction/IdService.java delete mode 100644 core/src/main/java/google/registry/persistence/transaction/ReplicaDbIdService.java diff --git a/core/src/main/java/google/registry/persistence/transaction/IdService.java b/core/src/main/java/google/registry/persistence/transaction/IdService.java deleted file mode 100644 index 77797789c66..00000000000 --- a/core/src/main/java/google/registry/persistence/transaction/IdService.java +++ /dev/null @@ -1,42 +0,0 @@ -// 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.tm; - -import java.util.concurrent.atomic.AtomicLong; - -/** - * Allocates a {@code long} to use as a {@code @Id}, (part) of the primary SQL key for an entity. - */ -final class IdService { - - private IdService() {} - - /** - * A SQL Sequence based ID allocator that generates an ID from a monotonically increasing {@link - * AtomicLong} - * - *

The generated IDs are project-wide unique. - */ - static long allocateId() { - return tm().transact( - () -> - (Long) - tm().getEntityManager() - .createNativeQuery("SELECT nextval('project_wide_unique_id_seq')") - .getSingleResult()); - } -} 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 8409b00aa32..f693630ea0d 100644 --- a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java +++ b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java @@ -70,6 +70,7 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.Callable; +import java.util.concurrent.atomic.AtomicLong; import java.util.function.Supplier; import java.util.stream.Stream; import java.util.stream.StreamSupport; @@ -224,7 +225,7 @@ public T transactNoRetry( EntityTransaction txn = txnInfo.entityManager.getTransaction(); try { txn.begin(); - txnInfo.start(clock, readOnly ? ReplicaDbIdService::allocatedId : IdService::allocateId); + txnInfo.start(clock, readOnly ? ReplicaDbIdService::allocateId : this::fetchIdFromSequence); if (readOnly) { getEntityManager().createNativeQuery("SET TRANSACTION READ ONLY").executeUpdate(); logger.atInfo().log("Using read-only SQL replica"); @@ -558,6 +559,19 @@ private EntityType getEntityType(Class clazz) { return emf.getMetamodel().entity(clazz); } + /** + * A SQL Sequence based ID allocator that generates an ID from a monotonically increasing {@link + * AtomicLong} + * + *

The generated IDs are project-wide unique. + */ + private long fetchIdFromSequence() { + return (Long) + getEntityManager() + .createNativeQuery("SELECT nextval('project_wide_unique_id_seq')") + .getSingleResult(); + } + private record EntityId(String name, Object value) {} private static ImmutableSet getEntityIdsFromEntity( @@ -1038,4 +1052,23 @@ public ImmutableList list() { .collect(toImmutableList()); } } + + /** + * Provides {@code long} values for use as {@code id} by JPA model entities in (read-only) + * transactions in the replica database. Each id is only unique in the JVM instance. + * + *

The {@link #fetchIdFromSequence database sequence-based id allocator} cannot be used with + * the replica because id generation is a write operation. + */ + private static final class ReplicaDbIdService { + + private static final AtomicLong nextId = new AtomicLong(1); + + /** + * Returns the next long value from a {@link AtomicLong}. Each id is unique in the JVM instance. + */ + static long allocateId() { + return nextId.getAndIncrement(); + } + } } diff --git a/core/src/main/java/google/registry/persistence/transaction/ReplicaDbIdService.java b/core/src/main/java/google/registry/persistence/transaction/ReplicaDbIdService.java deleted file mode 100644 index 30a99d674f8..00000000000 --- a/core/src/main/java/google/registry/persistence/transaction/ReplicaDbIdService.java +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2024 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 java.util.concurrent.atomic.AtomicLong; - -/** - * Provides {@code long} values for use as {@code id} by JPA model entities in (read-only) - * transactions in the replica database. Each id is only unique in the JVM instance. - * - *

The {@link IdService database sequence-based id service} cannot be used with the replica - * because id generation is a write operation. - */ -final class ReplicaDbIdService { - - private ReplicaDbIdService() {} - - private static final AtomicLong nextId = new AtomicLong(1); - - /** - * Returns the next long value from a {@link AtomicLong}. Each id is unique in the JVM instance. - */ - static final long allocatedId() { - return nextId.getAndIncrement(); - } -} From c47f82175460058156a08b4d91c2002f44748b31 Mon Sep 17 00:00:00 2001 From: Joaquin Gimenez Date: Wed, 18 Sep 2024 13:57:27 -0500 Subject: [PATCH 4/4] Fix typo in docs (#2520) --- docs/install.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/install.md b/docs/install.md index 14687a4fa89..d3cfcce0c2b 100644 --- a/docs/install.md +++ b/docs/install.md @@ -7,7 +7,7 @@ This document covers the steps necessary to download, build, and deploy Nomulus. You will need the following programs installed on your local machine: * A recent version of the [Java 11 JDK][java-jdk11]. -* [Google App Engine SDK for Java][app-engine-sdk], and configure aliases to to the `gcloud` and `appcfg.sh` utilities ( +* [Google App Engine SDK for Java][app-engine-sdk], and configure aliases to the `gcloud` and `appcfg.sh` utilities ( you'll use them a lot). * [Git](https://git-scm.com/) version control system. * Docker (confirm with `docker info` no permission issues, use `sudo groupadd docker` for sudoless docker).