Skip to content

Commit 17ff696

Browse files
author
Alexander Lavrukov
committed
review fix
1 parent 753e4bc commit 17ff696

File tree

4 files changed

+49
-44
lines changed

4 files changed

+49
-44
lines changed

‎repository/src/main/java/tech/ydb/yoj/repository/db/StdTxManager.java‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@
2020

2121
import javax.annotation.Nullable;
2222
import java.time.Duration;
23-
import java.util.Set;
2423
import java.util.concurrent.atomic.AtomicLong;
2524
import java.util.function.Supplier;
2625

26+
import static java.lang.String.format;
2727
import static java.util.Objects.requireNonNull;
2828
import static java.util.concurrent.TimeUnit.MILLISECONDS;
2929
import static tech.ydb.yoj.repository.db.IsolationLevel.ONLINE_CONSISTENT_READ_ONLY;
@@ -100,13 +100,13 @@ public StdTxManager(@NonNull Repository repository) {
100100
/* logContext */ null,
101101
/* options */ TxOptions.create(SERIALIZABLE_READ_WRITE),
102102
/* separatePolicy */ SeparatePolicy.LOG,
103-
/* txNameGenerator */ new TxNameGenerator.Default(Set.of())
103+
/* txNameGenerator */ new TxNameGenerator.Default()
104104
);
105105
}
106106

107107
@Override
108108
public StdTxManager withName(String name) {
109-
return withTxNameGenerator(new TxNameGenerator.Simple(name));
109+
return withTxNameGenerator(new TxNameGenerator.Constant(name));
110110
}
111111

112112
@Override
@@ -190,10 +190,10 @@ public void tx(Runnable runnable) {
190190

191191
@Override
192192
public <T> T tx(Supplier<T> supplier) {
193-
TxInfo txInfo = txNameGenerator.generate();
194-
String name = txInfo.name();
193+
TxName txName = txNameGenerator.generate();
194+
String name = txName.name();
195195

196-
checkSeparatePolicy(separatePolicy, txInfo.logName());
196+
checkSeparatePolicy(separatePolicy, txName.logName());
197197

198198
RetryableException lastRetryableException = null;
199199
TxImpl lastTx = null;
@@ -202,14 +202,14 @@ public <T> T tx(Supplier<T> supplier) {
202202
try {
203203
attempts.labels(name).observe(attempt);
204204
T result;
205-
RepositoryTransaction transaction = repository.startTransaction(options);
206-
lastTx = new TxImpl(name, transaction, options);
207205
try (
208206
var ignored1 = attemptDuration.labels(name).startTimer();
209-
var ignored2 = MDC.putCloseable("tx", formatTx(txInfo));
207+
var ignored2 = MDC.putCloseable("tx", formatTx(txName));
210208
var ignored3 = MDC.putCloseable("tx-id", formatTxId());
211-
var ignored4 = MDC.putCloseable("tx-name", txInfo.logName())
209+
var ignored4 = MDC.putCloseable("tx-name", txName.logName())
212210
) {
211+
RepositoryTransaction transaction = repository.startTransaction(options);
212+
lastTx = new TxImpl(name, transaction, options);
213213
result = lastTx.run(supplier);
214214
}
215215

@@ -255,7 +255,7 @@ private static void checkSeparatePolicy(SeparatePolicy separatePolicy, String tx
255255
case ALLOW -> {
256256
}
257257
case STRICT ->
258-
throw new IllegalStateException("Transaction was run when another transaction is active");
258+
throw new IllegalStateException(format("Transaction %s was run when another transaction is active", txName));
259259
case LOG ->
260260
log.warn("Transaction '{}' was run when another transaction is active. Perhaps unexpected behavior. " +
261261
"Use TxManager.separate() to avoid this message", txName);
@@ -266,8 +266,8 @@ private String getExceptionNameForMetric(RetryableException e) {
266266
return Strings.removeSuffix(e.getClass().getSimpleName(), "Exception");
267267
}
268268

269-
private String formatTx(TxInfo txInfo) {
270-
return formatTxId() + " {" + txInfo.logName() + (logContext != null ? "/" + logContext : "") + "}";
269+
private String formatTx(TxName txName) {
270+
return formatTxId() + " {" + txName.logName() + (logContext != null ? "/" + logContext : "") + "}";
271271
}
272272

273273
private String formatTxId() {

‎repository/src/main/java/tech/ydb/yoj/repository/db/TxInfo.java‎ renamed to ‎repository/src/main/java/tech/ydb/yoj/repository/db/TxName.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package tech.ydb.yoj.repository.db;
22

3-
public record TxInfo(
3+
public record TxName(
44
String name,
55
String logName
66
) {

‎repository/src/main/java/tech/ydb/yoj/repository/db/TxNameGenerator.java‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,19 @@
11
package tech.ydb.yoj.repository.db;
22

3-
import lombok.RequiredArgsConstructor;
3+
import tech.ydb.yoj.ExperimentalApi;
44

55
import java.util.Set;
66
import java.util.regex.Pattern;
77

88
/**
99
* Generates default name for a transaction, depending on the caller class and caller method names.
1010
*/
11+
@ExperimentalApi(issue = "https://github.com/ydb-platform/yoj-project/issues/188")
1112
public interface TxNameGenerator {
1213
/**
1314
* @return transaction name
1415
*/
15-
TxInfo generate();
16+
TxName generate();
1617

1718
/**
1819
* Generates short tx names of the form {@code ClNam#meNa} (from {@code package.name.ClassName[$InnerClassName]} and {@code methodName}).
@@ -24,7 +25,6 @@ public interface TxNameGenerator {
2425
*
2526
* <p>This is the classic YOJ default tx name generator, used since YOJ 1.0.0.
2627
*/
27-
@RequiredArgsConstructor
2828
final class Default implements TxNameGenerator {
2929
private static final Pattern PACKAGE_PATTERN = Pattern.compile(".*\\.");
3030
private static final Pattern INNER_CLASS_PATTERN_CLEAR = Pattern.compile("\\$.*");
@@ -33,8 +33,12 @@ final class Default implements TxNameGenerator {
3333

3434
private final Set<String> packagesToSkip;
3535

36+
public Default(String... packagesToSkip) {
37+
this.packagesToSkip = Set.of(packagesToSkip);
38+
}
39+
3640
@Override
37-
public TxInfo generate() {
41+
public TxName generate() {
3842
var stack = getStackResult(callStack, packagesToSkip);
3943

4044
return stack.map(frame -> {
@@ -53,15 +57,18 @@ public TxInfo generate() {
5357
* (from {@code package.name.ClassName[$InnerClassName]} and {@code methodName}).
5458
* Inner class names, including anonymous class names, are kept in the {@code Class.getName()} format ({@code $<inner class name>}).
5559
*/
56-
@RequiredArgsConstructor
5760
final class Long implements TxNameGenerator {
5861
private static final Pattern PACKAGE_PATTERN = Pattern.compile(".*\\.");
5962
private static final CallStack callStack = new CallStack();
6063

6164
private final Set<String> packagesToSkip;
6265

66+
public Long(String... packagesToSkip) {
67+
this.packagesToSkip = Set.of(packagesToSkip);
68+
}
69+
6370
@Override
64-
public TxInfo generate() {
71+
public TxName generate() {
6572
var stack = getStackResult(callStack, packagesToSkip);
6673

6774
return stack.map(frame -> {
@@ -72,9 +79,9 @@ public TxInfo generate() {
7279
}
7380
}
7481

75-
private static TxInfo buildDefaultTxInfo(String name, int lineNumber) {
82+
private static TxName buildDefaultTxInfo(String name, int lineNumber) {
7683
String logName = name + ":" + lineNumber;
77-
return new TxInfo(name, logName);
84+
return new TxName(name, logName);
7885
}
7986

8087
private static CallStack.FrameResult getStackResult(CallStack callStack, Set<String> packagesToSkip) {
@@ -83,16 +90,16 @@ private static CallStack.FrameResult getStackResult(CallStack callStack, Set<Str
8390
.skipPackages(packagesToSkip);
8491
}
8592

86-
final class Simple implements TxNameGenerator {
87-
private final TxInfo txInfo;
93+
final class Constant implements TxNameGenerator {
94+
private final TxName txName;
8895

89-
public Simple(String name) {
90-
this.txInfo = new TxInfo(name, name);
96+
public Constant(String name) {
97+
this.txName = new TxName(name, name);
9198
}
9299

93100
@Override
94-
public TxInfo generate() {
95-
return txInfo;
101+
public TxName generate() {
102+
return txName;
96103
}
97104
}
98105
}

‎repository/src/test/java/tech/ydb/yoj/repository/db/StdTxManagerTest.java‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,9 @@
1717

1818
import java.util.ArrayList;
1919
import java.util.List;
20-
import java.util.Set;
2120
import java.util.concurrent.atomic.AtomicInteger;
2221

2322
import static org.assertj.core.api.Assertions.assertThat;
24-
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
2523
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2624
import static org.mockito.ArgumentMatchers.any;
2725
import static org.mockito.Mockito.mock;
@@ -37,80 +35,80 @@ public class StdTxManagerTest {
3735

3836
@Test
3937
public void testDbChildPackage_FromStackWalker_Auto() {
40-
var name = new TestDbTxCaller(new TxNameGenerator.Default(Set.of())).getTxName();
38+
var name = new TestDbTxCaller(new TxNameGenerator.Default()).getTxName();
4139
assertThat(name).isEqualTo("TesDbTxCal#getTxNam");
4240
}
4341

4442
@Test
4543
public void testDbChildPackage_FromStackWalker_User() {
46-
var name = new TestDbTxCaller(new TxNameGenerator.Simple("qq")).getTxName();
44+
var name = new TestDbTxCaller(new TxNameGenerator.Constant("qq")).getTxName();
4745
assertThat(name).isEqualTo("qq");
4846
}
4947

5048
@Test
5149
public void testNotDbChildPackage_FromStackWalker_Auto() {
52-
var name = new TestTxCaller(new TxNameGenerator.Default(Set.of())).getTxName();
50+
var name = new TestTxCaller(new TxNameGenerator.Default()).getTxName();
5351
assertThat(name).isEqualTo("TesTxCal#getTxNam");
5452
}
5553

5654
@Test
5755
public void testNotDbChildPackage_FromStackWalker_Auto_SkipCallerPackage() {
58-
var name = new TestTxCaller(new TxNameGenerator.Default(Set.of(TestTxCaller.class.getPackageName())))
56+
var name = new TestTxCaller(new TxNameGenerator.Default(TestTxCaller.class.getPackageName()))
5957
.getTxName();
6058
assertThat(name).isEqualTo("StdTxManTes#testNotDbChiPac_FroStaWal_Aut_SkiCalPac");
6159
}
6260

6361
@Test
6462
public void testNotDbPackage_Same() {
65-
var nameOld = new TestTxCaller(new TxNameGenerator.Default(Set.of())).getTxName();
66-
var nameNew = new TestTxCaller(new TxNameGenerator.Default(Set.of())).getTxName();
63+
var nameOld = new TestTxCaller(new TxNameGenerator.Default()).getTxName();
64+
var nameNew = new TestTxCaller(new TxNameGenerator.Default()).getTxName();
6765
assertThat(nameNew).isEqualTo(nameOld);
6866
}
6967

7068
@Test
7169
public void testNotDbPackage_FromStackWalker_User() {
72-
var name = new TestTxCaller(new TxNameGenerator.Simple("omg")).getTxName();
70+
var name = new TestTxCaller(new TxNameGenerator.Constant("omg")).getTxName();
7371
assertThat(name).isEqualTo("omg");
7472
}
7573

7674
// explicitly TxNameGenerator.LONG
7775
@Test
7876
public void testDbChildPackage_FromStackWalker_Auto_Long() {
79-
var name = new TestDbTxCaller(new TxNameGenerator.Long(Set.of()))
77+
var name = new TestDbTxCaller(new TxNameGenerator.Long())
8078
.getTxName();
8179
assertThat(name).isEqualTo("TestDbTxCaller.getTxName");
8280
}
8381

8482
@Test
8583
public void testNotDbChildPackage_FromStackWalker_Auto_Long() {
86-
var name = new TestTxCaller(new TxNameGenerator.Long(Set.of()))
84+
var name = new TestTxCaller(new TxNameGenerator.Long())
8785
.getTxName();
8886
assertThat(name).isEqualTo("TestTxCaller.getTxName");
8987
}
9088

9189
@Test
9290
public void testNotDbChildPackage_FromStackWalker_Auto_Long_SkipCallerPackage() {
93-
var name = new TestTxCaller(new TxNameGenerator.Long(Set.of(TestTxCaller.class.getPackageName())))
91+
var name = new TestTxCaller(new TxNameGenerator.Long(TestTxCaller.class.getPackageName()))
9492
.getTxName();
9593
assertThat(name).isEqualTo("StdTxManagerTest.testNotDbChildPackage_FromStackWalker_Auto_Long_SkipCallerPackage");
9694
}
9795

9896
@Test
9997
public void testNotDbPackage_Same_Long() {
100-
var nameOld = new TestTxCaller(new TxNameGenerator.Long(Set.of())).getTxName();
101-
var nameNew = new TestTxCaller(new TxNameGenerator.Long(Set.of())).getTxName();
98+
var nameOld = new TestTxCaller(new TxNameGenerator.Long()).getTxName();
99+
var nameNew = new TestTxCaller(new TxNameGenerator.Long()).getTxName();
102100
assertThat(nameNew).isEqualTo(nameOld);
103101
}
104102

105103
@Test
106104
public void testNotDbPackage_ForceExplicitName_MustNotFail() {
107-
var name = new TestTxCaller(new TxNameGenerator.Simple("omg")).getTxName();
105+
var name = new TestTxCaller(new TxNameGenerator.Constant("omg")).getTxName();
108106
assertThat(name).isEqualTo("omg");
109107
}
110108

111109
@Test
112110
public void testDbPackage_ForceExplicitName_MustNotFail() {
113-
var name = new TestDbTxCaller(new TxNameGenerator.Simple("omg")).getTxName();
111+
var name = new TestDbTxCaller(new TxNameGenerator.Constant("omg")).getTxName();
114112
assertThat(name).isEqualTo("omg");
115113
}
116114

0 commit comments

Comments
 (0)