diff --git a/atlasdb-client/src/main/java/com/palantir/atlasdb/transaction/impl/ConflictDetectionManager.java b/atlasdb-client/src/main/java/com/palantir/atlasdb/transaction/impl/ConflictDetectionManager.java index 684f1dc2353..dfad9b39788 100644 --- a/atlasdb-client/src/main/java/com/palantir/atlasdb/transaction/impl/ConflictDetectionManager.java +++ b/atlasdb-client/src/main/java/com/palantir/atlasdb/transaction/impl/ConflictDetectionManager.java @@ -21,10 +21,11 @@ import com.palantir.atlasdb.keyvalue.api.TableReference; import com.palantir.atlasdb.transaction.api.ConflictHandler; import java.util.Map; +import java.util.Optional; import javax.annotation.Nullable; public class ConflictDetectionManager { - private final LoadingCache cache; + private final LoadingCache> cache; /** * This class does not make the mistake of attempting cache invalidation, @@ -38,20 +39,20 @@ public class ConflictDetectionManager { * * (This has always been the behavior of this class; I'm simply calling it out) */ - public ConflictDetectionManager(CacheLoader loader) { + public ConflictDetectionManager(CacheLoader> loader) { this.cache = CacheBuilder.newBuilder().maximumSize(100_000).build(loader); } - public void warmCacheWith(Map preload) { + public void warmCacheWith(Map> preload) { cache.putAll(preload); } - public Map getCachedValues() { + public Map> getCachedValues() { return cache.asMap(); } @Nullable - public ConflictHandler get(TableReference tableReference) { + public Optional get(TableReference tableReference) { return cache.getUnchecked(tableReference); } } diff --git a/atlasdb-client/src/main/java/com/palantir/atlasdb/transaction/impl/ConflictDetectionManagers.java b/atlasdb-client/src/main/java/com/palantir/atlasdb/transaction/impl/ConflictDetectionManagers.java index 01884847111..d574689061e 100644 --- a/atlasdb-client/src/main/java/com/palantir/atlasdb/transaction/impl/ConflictDetectionManagers.java +++ b/atlasdb-client/src/main/java/com/palantir/atlasdb/transaction/impl/ConflictDetectionManagers.java @@ -21,6 +21,7 @@ import com.palantir.atlasdb.keyvalue.api.TableReference; import com.palantir.atlasdb.table.description.TableMetadata; import com.palantir.atlasdb.transaction.api.ConflictHandler; +import java.util.Optional; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -30,10 +31,10 @@ public final class ConflictDetectionManagers { private ConflictDetectionManagers() {} public static ConflictDetectionManager createWithNoConflictDetection() { - return new ConflictDetectionManager(new CacheLoader() { + return new ConflictDetectionManager(new CacheLoader<>() { @Override - public ConflictHandler load(TableReference tableReference) throws Exception { - return ConflictHandler.IGNORE_ALL; + public Optional load(TableReference tableReference) throws Exception { + return Optional.of(ConflictHandler.IGNORE_ALL); } }); } @@ -63,21 +64,18 @@ public static ConflictDetectionManager create(KeyValueService kvs) { } private static ConflictDetectionManager create(KeyValueService kvs, boolean warmCache) { - ConflictDetectionManager conflictDetectionManager = - new ConflictDetectionManager(new CacheLoader() { - @Override - public ConflictHandler load(TableReference tableReference) throws Exception { - byte[] metadata = kvs.getMetadataForTable(tableReference); - if (metadata == null) { - log.error( - "Tried to make a transaction over a table that has no metadata: {}.", - tableReference); - return null; - } else { - return getConflictHandlerFromMetadata(metadata); - } - } - }); + ConflictDetectionManager conflictDetectionManager = new ConflictDetectionManager(new CacheLoader<>() { + @Override + public Optional load(TableReference tableReference) throws Exception { + byte[] metadata = kvs.getMetadataForTable(tableReference); + if (metadata == null) { + log.error("Tried to make a transaction over a table that has no metadata: {}.", tableReference); + return Optional.empty(); + } else { + return Optional.of(getConflictHandlerFromMetadata(metadata)); + } + } + }); if (warmCache) { // kick off an async thread that attempts to fully warm this cache // if it fails (e.g. probably this user has way too many tables), that's okay, @@ -91,9 +89,9 @@ public ConflictHandler load(TableReference tableReference) throws Exception { log.debug("Metadata was null for a table. likely because the table" + " is currently being created. Skipping warming" + " cache for the table."); - return null; + return Optional.empty(); } else { - return getConflictHandlerFromMetadata(metadata); + return Optional.of(getConflictHandlerFromMetadata(metadata)); } })); } catch (Throwable t) { diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TransactionConflictDetectionManager.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TransactionConflictDetectionManager.java index ed5aeeb5130..b973ec60ef9 100644 --- a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TransactionConflictDetectionManager.java +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TransactionConflictDetectionManager.java @@ -37,9 +37,10 @@ public TransactionConflictDetectionManager(ConflictDetectionManager delegate) { public void disableReadWriteConflict(TableReference tableRef) { conflictHandlers.compute(tableRef, (tableReference, nullableCurrentValue) -> { - ConflictHandler conflictHandler = delegate.get(tableReference); - Preconditions.checkNotNull(conflictHandler, "Conflict handler cannot be null when overwriting"); - Optional newValue = Optional.of(getDisabledReadWriteConflictHandler(conflictHandler)); + Optional conflictHandler = delegate.get(tableReference); + Preconditions.checkState(conflictHandler.isPresent(), "Conflict handler cannot be null when overwriting"); + Optional newValue = + Optional.of(getDisabledReadWriteConflictHandler(conflictHandler.get())); Preconditions.checkState( nullableCurrentValue == null || nullableCurrentValue.equals(newValue), @@ -68,8 +69,6 @@ private ConflictHandler getDisabledReadWriteConflictHandler(ConflictHandler conf @Nullable public ConflictHandler get(TableReference tableReference) { - return conflictHandlers - .computeIfAbsent(tableReference, tableRef -> Optional.ofNullable(delegate.get(tableRef))) - .orElse(null); + return conflictHandlers.computeIfAbsent(tableReference, delegate::get).orElse(null); } } diff --git a/atlasdb-impl-shared/src/test/java/com/palantir/atlasdb/transaction/impl/TransactionConflictDetectionManagerTest.java b/atlasdb-impl-shared/src/test/java/com/palantir/atlasdb/transaction/impl/TransactionConflictDetectionManagerTest.java index 0b9d861700d..f3c3f72b7fa 100644 --- a/atlasdb-impl-shared/src/test/java/com/palantir/atlasdb/transaction/impl/TransactionConflictDetectionManagerTest.java +++ b/atlasdb-impl-shared/src/test/java/com/palantir/atlasdb/transaction/impl/TransactionConflictDetectionManagerTest.java @@ -26,7 +26,7 @@ import com.palantir.atlasdb.keyvalue.api.TableReference; import com.palantir.atlasdb.transaction.api.ConflictHandler; import com.palantir.logsafe.exceptions.SafeIllegalStateException; -import com.palantir.logsafe.exceptions.SafeNullPointerException; +import java.util.Optional; import javax.annotation.Nullable; import org.junit.Before; import org.junit.Test; @@ -41,7 +41,7 @@ public final class TransactionConflictDetectionManagerTest { TableReference.create(Namespace.EMPTY_NAMESPACE, "test_table"); @Mock - private CacheLoader delegate; + private CacheLoader> delegate; private TransactionConflictDetectionManager conflictDetectionManager; @@ -50,7 +50,7 @@ public void before() { conflictDetectionManager = new TransactionConflictDetectionManager(new ConflictDetectionManager(delegate) { @Override @Nullable - public ConflictHandler get(TableReference tableReference) { + public Optional get(TableReference tableReference) { try { return delegate.load(tableReference); } catch (Exception e) { @@ -63,7 +63,7 @@ public ConflictHandler get(TableReference tableReference) { @Test public void testDisableReadWriteConflict_throwsIfDelegateValueIsNull() { assertThatLoggableExceptionThrownBy(() -> whenDisableReadWriteConflict(null)) - .isExactlyInstanceOf(SafeNullPointerException.class) + .isExactlyInstanceOf(SafeIllegalStateException.class) .hasLogMessage("Conflict handler cannot be null when overwriting"); } @@ -75,7 +75,7 @@ public void testDisableReadWriteConflict_multipleCalls() throws Exception { @Test public void testDisableReadWriteConflict_throwsIfCalledAfterTableWasUsed() throws Exception { - when(delegate.load(TABLE_REFERENCE)).thenReturn(ConflictHandler.SERIALIZABLE); + when(delegate.load(TABLE_REFERENCE)).thenReturn(Optional.of(ConflictHandler.SERIALIZABLE)); conflictDetectionManager.get(TABLE_REFERENCE); assertThatLoggableExceptionThrownBy(() -> conflictDetectionManager.disableReadWriteConflict(TABLE_REFERENCE)) .isExactlyInstanceOf(SafeIllegalStateException.class) @@ -130,7 +130,7 @@ private void testDisableReadWriteConflictThrowsUnsupported(ConflictHandler initi } private void whenDisableReadWriteConflict(ConflictHandler initial) throws Exception { - when(delegate.load(TABLE_REFERENCE)).thenReturn(initial); + when(delegate.load(TABLE_REFERENCE)).thenReturn(Optional.ofNullable(initial)); conflictDetectionManager.disableReadWriteConflict(TABLE_REFERENCE); } } diff --git a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/AbstractSerializableTransactionTest.java b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/AbstractSerializableTransactionTest.java index 48bfd0ba1ad..69b909941aa 100644 --- a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/AbstractSerializableTransactionTest.java +++ b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/AbstractSerializableTransactionTest.java @@ -129,11 +129,11 @@ protected Transaction startTransaction() { } private Transaction startTransactionWithOptions(TransactionOptions options) { - ImmutableMap tablesToWriteWrite = ImmutableMap.of( + ImmutableMap> tablesToWriteWrite = ImmutableMap.of( TEST_TABLE, - ConflictHandler.SERIALIZABLE, + Optional.of(ConflictHandler.SERIALIZABLE), TransactionConstants.TRANSACTION_TABLE, - ConflictHandler.IGNORE_ALL); + Optional.of(ConflictHandler.IGNORE_ALL)); return new SerializableTransaction( MetricsManagers.createForTests(), keyValueService, diff --git a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/CommitLockTest.java b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/CommitLockTest.java index f72db9a5973..5a7fca6dbbc 100644 --- a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/CommitLockTest.java +++ b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/CommitLockTest.java @@ -154,8 +154,11 @@ private void commitWriteWith(PreCommitCondition preCommitCondition, ConflictHand } private Transaction startTransaction(PreCommitCondition preCommitCondition, ConflictHandler conflictHandler) { - ImmutableMap tablesToWriteWrite = ImmutableMap.of( - TEST_TABLE, conflictHandler, TransactionConstants.TRANSACTION_TABLE, ConflictHandler.IGNORE_ALL); + ImmutableMap> tablesToWriteWrite = ImmutableMap.of( + TEST_TABLE, + Optional.of(conflictHandler), + TransactionConstants.TRANSACTION_TABLE, + Optional.of(ConflictHandler.IGNORE_ALL)); return new SerializableTransaction( MetricsManagers.createForTests(), keyValueService, diff --git a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TestConflictDetectionManagers.java b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TestConflictDetectionManagers.java index 9fd29e28287..f5befe9344f 100644 --- a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TestConflictDetectionManagers.java +++ b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TestConflictDetectionManagers.java @@ -20,16 +20,18 @@ import com.palantir.atlasdb.keyvalue.api.TableReference; import com.palantir.atlasdb.transaction.api.ConflictHandler; import java.util.Map; +import java.util.Optional; public final class TestConflictDetectionManagers { private TestConflictDetectionManagers() {} @VisibleForTesting - static ConflictDetectionManager createWithStaticConflictDetection(Map staticMap) { - return new ConflictDetectionManager(new CacheLoader() { + static ConflictDetectionManager createWithStaticConflictDetection( + Map> staticMap) { + return new ConflictDetectionManager(new CacheLoader<>() { @Override - public ConflictHandler load(TableReference tableReference) throws Exception { - return staticMap.getOrDefault(tableReference, ConflictHandler.RETRY_ON_WRITE_WRITE); + public Optional load(TableReference tableReference) throws Exception { + return staticMap.getOrDefault(tableReference, Optional.of(ConflictHandler.RETRY_ON_WRITE_WRITE)); } }); } diff --git a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TestTransactionManager.java b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TestTransactionManager.java index 50b32df655c..f8849d9e9cd 100644 --- a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TestTransactionManager.java +++ b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TestTransactionManager.java @@ -19,13 +19,14 @@ import com.palantir.atlasdb.transaction.api.ConflictHandler; import com.palantir.atlasdb.transaction.api.Transaction; import com.palantir.atlasdb.transaction.api.TransactionManager; +import java.util.Optional; public interface TestTransactionManager extends TransactionManager { Transaction commitAndStartNewTransaction(Transaction txn); Transaction createNewTransaction(); - void overrideConflictHandlerForTable(TableReference table, ConflictHandler conflictHandler); + void overrideConflictHandlerForTable(TableReference table, Optional conflictHandler); void setUnreadableTimestamp(long timestamp); } diff --git a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TestTransactionManagerImpl.java b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TestTransactionManagerImpl.java index 3b10658d72b..f7454dc08ef 100644 --- a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TestTransactionManagerImpl.java +++ b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TestTransactionManagerImpl.java @@ -52,7 +52,7 @@ public class TestTransactionManagerImpl extends SerializableTransactionManager i static final TransactionConfig TRANSACTION_CONFIG = ImmutableTransactionConfig.builder().build(); - private final Map conflictHandlerOverrides = new HashMap<>(); + private final Map> conflictHandlerOverrides = new HashMap<>(); private final WrapperWithTracker transactionWrapper; private final WrapperWithTracker keyValueServiceWrapper; private Optional unreadableTs = Optional.empty(); @@ -258,7 +258,7 @@ ConflictDetectionManager getConflictDetectionManager() { } @Override - public void overrideConflictHandlerForTable(TableReference table, ConflictHandler conflictHandler) { + public void overrideConflictHandlerForTable(TableReference table, Optional conflictHandler) { conflictHandlerOverrides.put(table, conflictHandler); } @@ -272,8 +272,8 @@ public void setUnreadableTimestamp(long timestamp) { unreadableTs = Optional.of(timestamp); } - private Map getConflictHandlerWithOverrides() { - Map conflictHandlersWithOverrides = new HashMap<>(); + private Map> getConflictHandlerWithOverrides() { + Map> conflictHandlersWithOverrides = new HashMap<>(); conflictHandlersWithOverrides.putAll(conflictDetectionManager.getCachedValues()); conflictHandlersWithOverrides.putAll(conflictHandlerOverrides); return conflictHandlersWithOverrides; diff --git a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/WrappingTestTransactionManager.java b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/WrappingTestTransactionManager.java index 25d37217a66..ea48c812a53 100644 --- a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/WrappingTestTransactionManager.java +++ b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/WrappingTestTransactionManager.java @@ -18,6 +18,7 @@ import com.palantir.atlasdb.keyvalue.api.TableReference; import com.palantir.atlasdb.transaction.api.ConflictHandler; import com.palantir.atlasdb.transaction.api.Transaction; +import java.util.Optional; public abstract class WrappingTestTransactionManager extends WrappingTransactionManager implements TestTransactionManager { @@ -39,7 +40,7 @@ public Transaction createNewTransaction() { } @Override - public void overrideConflictHandlerForTable(TableReference table, ConflictHandler conflictHandler) { + public void overrideConflictHandlerForTable(TableReference table, Optional conflictHandler) { delegate.overrideConflictHandlerForTable(table, conflictHandler); } diff --git a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/AtlasDbTestCase.java b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/AtlasDbTestCase.java index 3dd0d6adf79..371e8e31c78 100644 --- a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/AtlasDbTestCase.java +++ b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/AtlasDbTestCase.java @@ -48,6 +48,7 @@ import com.palantir.lock.LockService; import com.palantir.lock.impl.LockServiceImpl; import com.palantir.timestamp.InMemoryTimestampService; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutorService; import org.junit.After; @@ -137,7 +138,7 @@ public void tearDown() throws Exception { } protected void overrideConflictHandlerForTable(TableReference table, ConflictHandler conflictHandler) { - txManager.overrideConflictHandlerForTable(table, conflictHandler); + txManager.overrideConflictHandlerForTable(table, Optional.of(conflictHandler)); } protected void setConstraintCheckingMode(AtlasDbConstraintCheckingMode mode) { diff --git a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/SnapshotTransactionTest.java b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/SnapshotTransactionTest.java index 14ddc4877f3..01f35325367 100644 --- a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/SnapshotTransactionTest.java +++ b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/SnapshotTransactionTest.java @@ -2119,7 +2119,7 @@ private Transaction getSnapshotTransactionWith( NoOpCleaner.INSTANCE, startTs, TestConflictDetectionManagers.createWithStaticConflictDetection( - ImmutableMap.of(TABLE, ConflictHandler.RETRY_ON_WRITE_WRITE)), + ImmutableMap.of(TABLE, Optional.of(ConflictHandler.RETRY_ON_WRITE_WRITE))), SweepStrategyManagers.createDefault(keyValueService), lockImmutableTimestampResponse.getImmutableTimestamp(), Optional.of(lockImmutableTimestampResponse.getLock()), diff --git a/build.gradle b/build.gradle index bf3464c7fff..42864cac6c1 100644 --- a/build.gradle +++ b/build.gradle @@ -103,7 +103,6 @@ allprojects { options.compilerArgs += ['-Werror'] // temporarily relax constraints until we can fix all violations options.errorprone.disable 'ArrayAsKeyOfSetOrMap', - 'CacheLoaderNull', 'CloseableProvides', 'DangerousCompletableFutureUsage', 'DnsLookup',