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

Remove app engine deps from Lock (#1910)

Browse source
This commit is contained in:
Pavlo Tkach 2023-03-09 10:47:48 -05:00 • committed by GitHub
parent 025a2faff2
commit 438c523fcb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 15 additions and 350 deletions
Download patch file Download diff file Expand all files Collapse all files

44
core/src/main/java/google/registry/model/server/Lock.java
View file

@ -25,8 +25,6 @@ import com.google.common.base.Strings;
import com.google.common.flogger.FluentLogger;
import google.registry.model.ImmutableObject;
import google.registry.persistence.VKey;
import google.registry.util.RequestStatusChecker;
import google.registry.util.RequestStatusCheckerImpl;
import java.io.Serializable;
import java.util.Optional;
import java.util.function.Supplier;
@ -70,8 +68,7 @@ public class Lock extends ImmutableObject implements Serializable {
enum LockState {
IN_USE,
FREE,
TIMED_OUT,
OWNER_DIED
TIMED_OUT
}
@VisibleForTesting static LockMetrics lockMetrics = new LockMetrics();
@ -79,17 +76,6 @@ public class Lock extends ImmutableObject implements Serializable {
/** The name of the locked resource. */
@Transient @Id String lockId;
/**
* Unique log ID of the request that owns this lock.
*
* <p>When that request is no longer running (is finished), the lock can be considered implicitly
* released.
*
* <p>See {@link RequestStatusCheckerImpl#getLogId} for details about how it's created in
* practice.
*/
@Column String requestLogId;
/** When the lock can be considered implicitly released. */
@Column(nullable = false)
DateTime expirationTime;
@ -124,7 +110,6 @@ public class Lock extends ImmutableObject implements Serializable {
private static Lock create(
String resourceName,
String scope,
String requestLogId,
DateTime acquiredTime,
Duration leaseLength) {
checkArgument(!Strings.isNullOrEmpty(resourceName), "resourceName cannot be null or empty");
@ -132,7 +117,6 @@ public class Lock extends ImmutableObject implements Serializable {
// Add the scope to the Lock's id so that it is unique for locks acquiring the same resource
// across different TLDs.
instance.lockId = makeLockId(resourceName, scope);
instance.requestLogId = requestLogId;
instance.expirationTime = acquiredTime.plus(leaseLength);
instance.acquiredTime = acquiredTime;
instance.resourceName = resourceName;
@ -172,18 +156,13 @@ public class Lock extends ImmutableObject implements Serializable {
switch (acquireResult.lockState()) {
case IN_USE:
logger.atInfo().log(
"Existing lock by request %s is still valid now %s (until %s) lock: %s",
lock.requestLogId, now, lock.expirationTime, lock.lockId);
"Existing lock by request is still valid now %s (until %s) lock: %s",
now, lock.expirationTime, lock.lockId);
break;
case TIMED_OUT:
logger.atInfo().log(
"Existing lock by request %s is timed out now %s (was valid until %s) lock: %s",
lock.requestLogId, now, lock.expirationTime, lock.lockId);
break;
case OWNER_DIED:
logger.atInfo().log(
"Existing lock is valid now %s (until %s), but owner (%s) isn't running lock: %s",
now, lock.expirationTime, lock.requestLogId, lock.lockId);
"Existing lock by request is timed out now %s (was valid until %s) lock: %s",
now, lock.expirationTime, lock.lockId);
break;
case FREE:
// There was no existing lock
@ -203,11 +182,7 @@ public class Lock extends ImmutableObject implements Serializable {
/** Try to acquire a lock. Returns absent if it can't be acquired. */
public static Optional<Lock> acquire(
String resourceName,
@Nullable String tld,
Duration leaseLength,
RequestStatusChecker requestStatusChecker,
boolean checkThreadRunning) {
String resourceName, @Nullable String tld, Duration leaseLength) {
String scope = tld != null ? tld : GLOBAL;
Supplier<AcquireResult> lockAcquirer =
() -> {
@ -219,22 +194,19 @@ public class Lock extends ImmutableObject implements Serializable {
.orElse(null);
if (lock != null) {
logger.atInfo().log(
"Loaded existing lock: %s for request: %s", lock.lockId, lock.requestLogId);
"Loaded existing lock: %s for resource: %s", lock.lockId, lock.resourceName);
}
LockState lockState;
if (lock == null) {
lockState = LockState.FREE;
} else if (isAtOrAfter(now, lock.expirationTime)) {
lockState = LockState.TIMED_OUT;
} else if (checkThreadRunning && !requestStatusChecker.isRunning(lock.requestLogId)) {
lockState = LockState.OWNER_DIED;
} else {
lockState = LockState.IN_USE;
return AcquireResult.create(now, lock, null, lockState);
}
Lock newLock =
create(resourceName, scope, requestStatusChecker.getLogId(), now, leaseLength);
Lock newLock = create(resourceName, scope, now, leaseLength);
tm().put(newLock);
return AcquireResult.create(now, lock, newLock, lockState);

14
core/src/main/java/google/registry/request/RequestModule.java
View file

@ -39,8 +39,6 @@ import google.registry.request.HttpException.UnsupportedMediaTypeException;
import google.registry.request.auth.AuthResult;
import google.registry.request.lock.LockHandler;
import google.registry.request.lock.LockHandlerImpl;
import google.registry.util.RequestStatusChecker;
import google.registry.util.RequestStatusCheckerImpl;
import java.io.IOException;
import java.util.Map;
import java.util.Optional;
@ -192,18 +190,6 @@ public final class RequestModule {
return lockHandler;
}
@Provides
static RequestStatusChecker provideRequestStatusChecker(
RequestStatusCheckerImpl requestStatusChecker) {
return requestStatusChecker;
}
@Provides
@RequestLogId
static String provideRequestLogId(RequestStatusChecker requestStatusChecker) {
return requestStatusChecker.getLogId();
}
@Provides
@JsonPayload
@SuppressWarnings("unchecked")

7
core/src/main/java/google/registry/request/lock/LockHandlerImpl.java
View file

@ -26,7 +26,6 @@ import com.google.common.util.concurrent.UncheckedExecutionException;
import google.registry.model.server.Lock;
import google.registry.util.AppEngineTimeLimiter;
import google.registry.util.Clock;
import google.registry.util.RequestStatusChecker;
import java.util.HashSet;
import java.util.Optional;
import java.util.Set;
@ -48,12 +47,10 @@ public class LockHandlerImpl implements LockHandler {
/** Fudge factor to make sure we kill threads before a lock actually expires. */
private static final Duration LOCK_TIMEOUT_FUDGE = Duration.standardSeconds(5);
private final RequestStatusChecker requestStatusChecker;
private final Clock clock;
@Inject
public LockHandlerImpl(RequestStatusChecker requestStatusChecker, Clock clock) {
this.requestStatusChecker = requestStatusChecker;
public LockHandlerImpl(Clock clock) {
this.clock = clock;
}
@ -114,7 +111,7 @@ public class LockHandlerImpl implements LockHandler {
/** Allows injection of mock Lock in tests. */
@VisibleForTesting
Optional<Lock> acquire(String lockName, @Nullable String tld, Duration leaseLength) {
return Lock.acquire(lockName, tld, leaseLength, requestStatusChecker, true);
return Lock.acquire(lockName, tld, leaseLength);
}
private interface LockAcquirer {

15
core/src/test/java/google/registry/model/server/LockTest.java
View file

@ -18,17 +18,14 @@ import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
import static google.registry.model.server.Lock.LockState.FREE;
import static google.registry.model.server.Lock.LockState.IN_USE;
import static google.registry.model.server.Lock.LockState.OWNER_DIED;
import static google.registry.model.server.Lock.LockState.TIMED_OUT;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
import google.registry.model.EntityTestCase;
import google.registry.model.server.Lock.LockState;
import google.registry.util.RequestStatusChecker;
import java.util.Optional;
import org.joda.time.Duration;
import org.junit.jupiter.api.AfterEach;
@ -41,7 +38,6 @@ public class LockTest extends EntityTestCase {
private static final String RESOURCE_NAME = "foo";
private static final Duration ONE_DAY = Duration.standardDays(1);
private static final Duration TWO_MILLIS = Duration.millis(2);
private static final RequestStatusChecker requestStatusChecker = mock(RequestStatusChecker.class);
private LockMetrics origLockMetrics;
@ -52,7 +48,7 @@ public class LockTest extends EntityTestCase {
private static Optional<Lock> acquire(
String tld, Duration leaseLength, LockState expectedLockState) {
Lock.lockMetrics = mock(LockMetrics.class);
Optional<Lock> lock = Lock.acquire(RESOURCE_NAME, tld, leaseLength, requestStatusChecker, true);
Optional<Lock> lock = Lock.acquire(RESOURCE_NAME, tld, leaseLength);
verify(Lock.lockMetrics).recordAcquire(RESOURCE_NAME, tld, expectedLockState);
verifyNoMoreInteractions(Lock.lockMetrics);
Lock.lockMetrics = null;
@ -72,8 +68,6 @@ public class LockTest extends EntityTestCase {
void beforeEach() {
origLockMetrics = Lock.lockMetrics;
Lock.lockMetrics = null;
when(requestStatusChecker.getLogId()).thenReturn("current-request-id");
when(requestStatusChecker.isRunning("current-request-id")).thenReturn(true);
}
@AfterEach
@ -111,9 +105,6 @@ public class LockTest extends EntityTestCase {
assertThat(acquire("", ONE_DAY, FREE)).isPresent();
// We can't get it again while request is active
assertThat(acquire("", ONE_DAY, IN_USE)).isEmpty();
// But if request is finished, we can get it.
when(requestStatusChecker.isRunning("current-request-id")).thenReturn(false);
assertThat(acquire("", ONE_DAY, OWNER_DIED)).isPresent();
}
@Test
@ -134,9 +125,7 @@ public class LockTest extends EntityTestCase {
@Test
void testFailure_emptyResourceName() {
IllegalArgumentException thrown =
assertThrows(
IllegalArgumentException.class,
() -> Lock.acquire("", "", TWO_MILLIS, requestStatusChecker, true));
assertThrows(IllegalArgumentException.class, () -> Lock.acquire("", "", TWO_MILLIS));
assertThat(thrown).hasMessageThat().contains("resourceName cannot be null or empty");
}
}

3
core/src/test/java/google/registry/request/lock/LockHandlerImplTest.java
View file

@ -23,7 +23,6 @@ import static org.mockito.Mockito.verify;
import google.registry.model.server.Lock;
import google.registry.testing.FakeClock;
import google.registry.testing.UserServiceExtension;
import google.registry.util.RequestStatusCheckerImpl;
import java.util.Optional;
import java.util.concurrent.Callable;
import java.util.concurrent.TimeoutException;
@ -134,7 +133,7 @@ final class LockHandlerImplTest {
}
private LockHandler createTestLockHandler(@Nullable Lock acquiredLock) {
return new LockHandlerImpl(new RequestStatusCheckerImpl(), clock) {
return new LockHandlerImpl(clock) {
private static final long serialVersionUID = 0L;
@Override

129
core/src/test/java/google/registry/util/RequestStatusCheckerImplTest.java
View file

@ -1,129 +0,0 @@
// 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.util;
import static com.google.common.truth.Truth.assertThat;
import static google.registry.testing.LogsSubject.assertAboutLogs;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import com.google.appengine.api.log.LogQuery;
import com.google.appengine.api.log.LogService;
import com.google.appengine.api.log.RequestLogs;
import com.google.apphosting.api.ApiProxy;
import com.google.common.collect.ImmutableList;
import com.google.common.testing.TestLogHandler;
import google.registry.testing.UserServiceExtension;
import java.util.logging.Level;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
/** Unit tests for {@link RequestStatusCheckerImpl}. */
final class RequestStatusCheckerImplTest {
private static final TestLogHandler logHandler = new TestLogHandler();
private static final RequestStatusChecker requestStatusChecker = new RequestStatusCheckerImpl();
/**
* Matcher for the expected LogQuery in {@link RequestStatusCheckerImpl#isRunning}.
*
* Because LogQuery doesn't have a .equals function, we have to create an actual matcher to make
* sure we have the right argument in our mocks.
*/
private static LogQuery expectedLogQuery(final String requestLogId) {
return argThat(
object -> {
assertThat(object).isInstanceOf(LogQuery.class);
assertThat(object.getRequestIds()).containsExactly(requestLogId);
assertThat(object.getIncludeAppLogs()).isFalse();
assertThat(object.getIncludeIncomplete()).isTrue();
return true;
});
}
// We do not actually need to set up user service, rather, we just need this extension to set up
// App Engine environment so the status checker can make an App Engine API call.
@RegisterExtension UserServiceExtension userService = new UserServiceExtension("");
@BeforeEach
void beforeEach() {
JdkLoggerConfig.getConfig(RequestStatusCheckerImpl.class).addHandler(logHandler);
RequestStatusCheckerImpl.logService = mock(LogService.class);
}
@AfterEach
void afterEach() {
JdkLoggerConfig.getConfig(RequestStatusCheckerImpl.class).removeHandler(logHandler);
}
// If a logId is unrecognized, it could be that the log hasn't been uploaded yet - so we assume
// it's a request that has just started running recently.
@Test
void testIsRunning_unrecognized() {
when(RequestStatusCheckerImpl.logService.fetch(expectedLogQuery("12345678")))
.thenReturn(ImmutableList.of());
assertThat(requestStatusChecker.isRunning("12345678")).isTrue();
assertAboutLogs()
.that(logHandler)
.hasLogAtLevelWithMessage(Level.INFO, "Queried an unrecognized requestLogId");
}
@Test
void testIsRunning_notFinished() {
RequestLogs requestLogs = new RequestLogs();
requestLogs.setFinished(false);
when(RequestStatusCheckerImpl.logService.fetch(expectedLogQuery("12345678")))
.thenReturn(ImmutableList.of(requestLogs));
assertThat(requestStatusChecker.isRunning("12345678")).isTrue();
assertAboutLogs()
.that(logHandler)
.hasLogAtLevelWithMessage(Level.INFO, "isFinished: false");
}
@Test
void testIsRunning_finished() {
RequestLogs requestLogs = new RequestLogs();
requestLogs.setFinished(true);
when(RequestStatusCheckerImpl.logService.fetch(expectedLogQuery("12345678")))
.thenReturn(ImmutableList.of(requestLogs));
assertThat(requestStatusChecker.isRunning("12345678")).isFalse();
assertAboutLogs()
.that(logHandler)
.hasLogAtLevelWithMessage(Level.INFO, "isFinished: true");
}
@Test
void testGetLogId_returnsRequestLogId() {
String expectedLogId = ApiProxy.getCurrentEnvironment().getAttributes().get(
"com.google.appengine.runtime.request_log_id").toString();
assertThat(requestStatusChecker.getLogId()).isEqualTo(expectedLogId);
}
@Test
void testGetLogId_createsLog() {
requestStatusChecker.getLogId();
assertAboutLogs()
.that(logHandler)
.hasLogAtLevelWithMessage(Level.INFO, "Current requestLogId: ");
}
}