From ca389b249a6245df0c50a2a618153b1330e1b585 Mon Sep 17 00:00:00 2001 From: Nikolai Amelichev Date: Fri, 1 Nov 2024 17:11:27 +0100 Subject: [PATCH] Make StdTxManager.useNewTxNameGeneration do nothing and deprecate it for removal --- .../yoj/databind/expression/FieldValue.java | 16 +++++++ .../ydb/yoj/repository/db/StdTxManager.java | 41 ++++++++--------- .../yoj/repository/db/StdTxManagerTest.java | 45 ++++--------------- .../db/testcaller/TestDbTxCaller.java | 19 +++----- .../repository/testcaller/TestTxCaller.java | 19 +++----- 5 files changed, 54 insertions(+), 86 deletions(-) diff --git a/databind/src/main/java/tech/ydb/yoj/databind/expression/FieldValue.java b/databind/src/main/java/tech/ydb/yoj/databind/expression/FieldValue.java index f1bedfce..df942e1b 100644 --- a/databind/src/main/java/tech/ydb/yoj/databind/expression/FieldValue.java +++ b/databind/src/main/java/tech/ydb/yoj/databind/expression/FieldValue.java @@ -159,6 +159,8 @@ default boolean isUuid() { */ @NonNull static FieldValue ofStr(@NonNull String str) { + DeprecationWarnings.warnOnce("FieldValue.ofStr", + "Please use new tech.ydb.yoj.databind.expression.values.StringFieldValue(str)"); return new StringFieldValue(str); } @@ -171,6 +173,8 @@ static FieldValue ofStr(@NonNull String str) { */ @NonNull static FieldValue ofNum(long num) { + DeprecationWarnings.warnOnce("FieldValue.ofNum", + "Please use new tech.ydb.yoj.databind.expression.values.IntegerFieldValue(num)"); return new IntegerFieldValue(num); } @@ -183,6 +187,8 @@ static FieldValue ofNum(long num) { */ @NonNull static FieldValue ofReal(double real) { + DeprecationWarnings.warnOnce("FieldValue.ofReal", + "Please use new tech.ydb.yoj.databind.expression.values.RealFieldValue(real)"); return new RealFieldValue(real); } @@ -195,6 +201,8 @@ static FieldValue ofReal(double real) { */ @NonNull static FieldValue ofBool(boolean bool) { + DeprecationWarnings.warnOnce("FieldValue.ofBool", + "Please use new tech.ydb.yoj.databind.expression.values.BooleanFieldValue(bool)"); return new BooleanFieldValue(bool); } @@ -207,6 +215,8 @@ static FieldValue ofBool(boolean bool) { */ @NonNull static FieldValue ofTimestamp(@NonNull Instant timestamp) { + DeprecationWarnings.warnOnce("FieldValue.ofTimestamp", + "Please use new tech.ydb.yoj.databind.expression.values.TimestampFieldValue(timestamp)"); return new TimestampFieldValue(timestamp); } @@ -218,6 +228,8 @@ static FieldValue ofTimestamp(@NonNull Instant timestamp) { */ @NonNull static FieldValue ofTuple(@NonNull Tuple tuple) { + DeprecationWarnings.warnOnce("FieldValue.ofTuple", + "Please use new tech.ydb.yoj.databind.expression.values.TupleFieldValue(tuple)"); return new TupleFieldValue(tuple); } @@ -230,6 +242,8 @@ static FieldValue ofTuple(@NonNull Tuple tuple) { */ @NonNull static FieldValue ofByteArray(@NonNull ByteArray byteArray) { + DeprecationWarnings.warnOnce("FieldValue.ofByteArray", + "Please use new tech.ydb.yoj.databind.expression.values.ByteArrayFieldValue(byteArray)"); return new ByteArrayFieldValue(byteArray); } @@ -242,6 +256,8 @@ static FieldValue ofByteArray(@NonNull ByteArray byteArray) { */ @NonNull static FieldValue ofUuid(@NonNull UUID uuid) { + DeprecationWarnings.warnOnce("FieldValue.ofUuid", + "Please use new tech.ydb.yoj.databind.expression.values.UuidFieldValue(uuid)"); return new UuidFieldValue(uuid); } } diff --git a/repository/src/main/java/tech/ydb/yoj/repository/db/StdTxManager.java b/repository/src/main/java/tech/ydb/yoj/repository/db/StdTxManager.java index 49bb2407..ec72b1cf 100644 --- a/repository/src/main/java/tech/ydb/yoj/repository/db/StdTxManager.java +++ b/repository/src/main/java/tech/ydb/yoj/repository/db/StdTxManager.java @@ -13,6 +13,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.slf4j.MDC; +import tech.ydb.yoj.DeprecationWarnings; import tech.ydb.yoj.repository.db.cache.TransactionLog; import tech.ydb.yoj.repository.db.exception.RetryableException; import tech.ydb.yoj.util.lang.CallStack; @@ -45,13 +46,18 @@ */ @RequiredArgsConstructor(access = AccessLevel.PRIVATE) public final class StdTxManager implements TxManager, TxManagerState { - public static volatile boolean useNewTxNameGeneration = false; + /** + * @deprecated Please stop using the {@code StdTxManager.useNewTxNameGeneration} field. + * Changing this field has no effect as of YOJ 2.6.1, and it will be removed completely in YOJ 3.0.0. + */ + @Deprecated(forRemoval = true) + public static volatile boolean useNewTxNameGeneration = true; private static final Logger log = LoggerFactory.getLogger(StdTxManager.class); private static final CallStack callStack = new CallStack(); - private static final int DEFAULT_MAX_ATTEMPT_COUNT = 100; + private static final int DEFAULT_MAX_ATTEMPT_COUNT = 10; private static final double[] TX_ATTEMPTS_BUCKETS = new double[] {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12, 14, 16, 18, 20, 25, 35, 40, 45, 50, 60, 70, 80, 90, 100}; private static final double[] DURATION_BUCKETS = { @@ -111,11 +117,14 @@ public StdTxManager(Repository repository) { } /** - * @deprecated Low-level constructor, kept for compatibility with existing code. Not recommended. + * @deprecated Constructor is in YOJ 2.x for backwards compatibility, an will be removed in YOJ 3.0.0. Please construct + * {@link #StdTxManager(Repository)} and customize it using the {@code with<...>()} methods instead. */ - @Deprecated + @Deprecated(forRemoval = true) public StdTxManager(Repository repository, int maxAttemptCount, String name, Integer logLine, String logContext, TxOptions options) { this(repository, maxAttemptCount, name, logLine, logContext, options, SeparatePolicy.LOG, Set.of()); + DeprecationWarnings.warnOnce("StdTxManager(Repository, int, String, Integer, String, TxOptions)", + "Please use the recommended StdTxManager(Repository) constructor and customize the TxManager by using with<...>() methods"); } @Override @@ -267,28 +276,14 @@ private T runAttempt(Supplier supplier, TxImpl tx) { } private StdTxManager withGeneratedNameAndLine() { - return useNewTxNameGeneration ? withGeneratedNameAndLineNew() : withGeneratedNameAndLineOld(); - } - - private StdTxManager withGeneratedNameAndLineOld() { - String pkg = StdTxManager.class.getPackage().getName(); - for (StackTraceElement ste : new Exception().getStackTrace()) { - if (!ste.getClassName().startsWith(pkg)) { - String name = ste.getClassName() - .replaceFirst(".*\\.", "") - .replaceFirst("\\$.*", "") - .replaceAll("([A-Z][a-z]{2})[a-z]+", "$1") - + "#" + ste.getMethodName() - .replaceAll("([A-Z][a-z]{2})[a-z]+", "$1"); - return withName(name).withLogLine(ste.getLineNumber()); - } + record TxInfo(String name, int lineNumber) { } - throw new IllegalStateException("Stacktrace doesn't contain elements from other packages"); - } - private StdTxManager withGeneratedNameAndLineNew() { - record TxInfo(String name, int lineNumber) { + if (!useNewTxNameGeneration) { + DeprecationWarnings.warnOnce("StdTxManager.useNewTxNameGeneration", + "As of YOJ 2.6.1, setting StdTxManager.useNewTxNameGeneration has no effect. Please stop setting this field"); } + var info = callStack.findCallingFrame() .skipPackage(StdTxManager.class.getPackageName()) .skipPackages(skipCallerPackages) diff --git a/repository/src/test/java/tech/ydb/yoj/repository/db/StdTxManagerTest.java b/repository/src/test/java/tech/ydb/yoj/repository/db/StdTxManagerTest.java index de79c043..f8da0a7b 100644 --- a/repository/src/test/java/tech/ydb/yoj/repository/db/StdTxManagerTest.java +++ b/repository/src/test/java/tech/ydb/yoj/repository/db/StdTxManagerTest.java @@ -34,69 +34,40 @@ public class StdTxManagerTest { private final TransactionLog transactionLog = new TransactionLog(TransactionLog.Level.DEBUG); private final RepositoryTransaction repositoryTransaction = mock(RepositoryTransaction.class); - @Test - public void testDbChildPackage_FromException_Auto() { - var name = new TestDbTxCaller(false, null).getTxName(); - assertThat(name).isIn( - // JDK 17 and below - "NatMetAccImp#invoke0", - // JDK 19 - "DirMetHanAcc#invoke" - ); - } - - @Test - public void testDbChildPackage_FromException_User() { - var name = new TestDbTxCaller(false, "omg").getTxName(); - assertThat(name).isEqualTo("omg"); - } - @Test public void testDbChildPackage_FromStackWalker_Auto() { - var name = new TestDbTxCaller(true, null).getTxName(); + var name = new TestDbTxCaller(null).getTxName(); assertThat(name).isEqualTo("TesDbTxCal#getTxNam"); } @Test public void testDbChildPackage_FromStackWalker_User() { - var name = new TestDbTxCaller(true, "qq").getTxName(); + var name = new TestDbTxCaller("qq").getTxName(); assertThat(name).isEqualTo("qq"); } - @Test - public void testNotDbChildPackage_FromException_Auto() { - var name = new TestTxCaller(false, null).getTxName(); - assertThat(name).isEqualTo("TesTxCal#getTxNam"); - } - @Test public void testNotDbChildPackage_FromStackWalker_Auto() { - var name = new TestTxCaller(true, null).getTxName(); + var name = new TestTxCaller(null).getTxName(); assertThat(name).isEqualTo("TesTxCal#getTxNam"); } @Test public void testNotDbChildPackage_FromStackWalker_Auto_SkipCallerPackage() { - var name = new TestTxCaller(true, null, Set.of(TestTxCaller.class.getPackageName())).getTxName(); + var name = new TestTxCaller(null, Set.of(TestTxCaller.class.getPackageName())).getTxName(); assertThat(name).isEqualTo("StdTxManTes#testNotDbChiPac_FroStaWal_Aut_SkiCalPac"); } @Test public void testNotDbPackage_Same() { - var nameOld = new TestTxCaller(false, null).getTxName(); - var nameNew = new TestTxCaller(true, null).getTxName(); + var nameOld = new TestTxCaller(null).getTxName(); + var nameNew = new TestTxCaller(null).getTxName(); assertThat(nameNew).isEqualTo(nameOld); } - @Test - public void testNotDbPackage_FromException_User() { - var name = new TestTxCaller(false, "omg").getTxName(); - assertThat(name).isEqualTo("omg"); - } - @Test public void testNotDbPackage_FromStackWalker_User() { - var name = new TestTxCaller(true, "omg").getTxName(); + var name = new TestTxCaller("omg").getTxName(); assertThat(name).isEqualTo("omg"); } @@ -218,7 +189,7 @@ public void testDryRun_True_ThrowRuntimeException() { private static final class TestAppender extends AbstractAppender { private final List messages = new ArrayList<>(); - protected TestAppender(String name) { + private TestAppender(String name) { super(name, null, null, true, null); } diff --git a/repository/src/test/java/tech/ydb/yoj/repository/db/testcaller/TestDbTxCaller.java b/repository/src/test/java/tech/ydb/yoj/repository/db/testcaller/TestDbTxCaller.java index e289b307..a48a5ea2 100644 --- a/repository/src/test/java/tech/ydb/yoj/repository/db/testcaller/TestDbTxCaller.java +++ b/repository/src/test/java/tech/ydb/yoj/repository/db/testcaller/TestDbTxCaller.java @@ -23,12 +23,11 @@ */ @RequiredArgsConstructor public class TestDbTxCaller { - private final boolean useNewTxNameGeneration; private final String explicitName; private final Set skipCallerPackages; - public TestDbTxCaller(boolean useNewTxNameGeneration, String explicitName) { - this(useNewTxNameGeneration, explicitName, Set.of()); + public TestDbTxCaller(String explicitName) { + this(explicitName, Set.of()); } public String getTxName() { @@ -37,16 +36,10 @@ public String getTxName() { when(rt.getTransactionLocal()).thenReturn(new TransactionLocal(TxOptions.create(IsolationLevel.SERIALIZABLE_READ_WRITE))); when(repo.startTransaction(any(TxOptions.class))).thenReturn(rt); - var txGenerationBackup = StdTxManager.useNewTxNameGeneration; - StdTxManager.useNewTxNameGeneration = useNewTxNameGeneration; - try { - var tx = new StdTxManager(repo).withSkipCallerPackages(skipCallerPackages); - if (explicitName != null) { - tx = tx.withName(explicitName); - } - return tx.tx(() -> Tx.Current.get().getName()); - } finally { - StdTxManager.useNewTxNameGeneration = txGenerationBackup; + var tx = new StdTxManager(repo).withSkipCallerPackages(skipCallerPackages); + if (explicitName != null) { + tx = tx.withName(explicitName); } + return tx.tx(() -> Tx.Current.get().getName()); } } diff --git a/repository/src/test/java/tech/ydb/yoj/repository/testcaller/TestTxCaller.java b/repository/src/test/java/tech/ydb/yoj/repository/testcaller/TestTxCaller.java index 6300ed65..ec41ef21 100644 --- a/repository/src/test/java/tech/ydb/yoj/repository/testcaller/TestTxCaller.java +++ b/repository/src/test/java/tech/ydb/yoj/repository/testcaller/TestTxCaller.java @@ -23,12 +23,11 @@ */ @RequiredArgsConstructor public class TestTxCaller { - private final boolean useNewTxNameGeneration; private final String explicitName; private final Set skipCallerPackages; - public TestTxCaller(boolean useNewTxNameGeneration, String explicitName) { - this(useNewTxNameGeneration, explicitName, Set.of()); + public TestTxCaller(String explicitName) { + this(explicitName, Set.of()); } public String getTxName() { @@ -37,16 +36,10 @@ public String getTxName() { when(rt.getTransactionLocal()).thenReturn(new TransactionLocal(TxOptions.create(IsolationLevel.SERIALIZABLE_READ_WRITE))); when(repo.startTransaction(any(TxOptions.class))).thenReturn(rt); - var txGenerationBackup = StdTxManager.useNewTxNameGeneration; - StdTxManager.useNewTxNameGeneration = useNewTxNameGeneration; - try { - var tx = new StdTxManager(repo).withSkipCallerPackages(skipCallerPackages); - if (explicitName != null) { - tx = tx.withName(explicitName); - } - return tx.tx(() -> Tx.Current.get().getName()); - } finally { - StdTxManager.useNewTxNameGeneration = txGenerationBackup; + var tx = new StdTxManager(repo).withSkipCallerPackages(skipCallerPackages); + if (explicitName != null) { + tx = tx.withName(explicitName); } + return tx.tx(() -> Tx.Current.get().getName()); } }