Skip to content

Commit 8b58c55

Browse files
committed
test(rpc): drop probe tests that guard nothing
Three of the tests written while designing the cursor interceptor were exploration scaffolding, not regression guards: CursorInterceptorAttachmentTest drove probe services only, so switching the production code back to server-level attachment left it green; the wiring it was meant to justify is now covered by CursorInterceptorWiringTest, and the reasoning belongs in the PR. WalletSolidityApiMethodSubsetTest asserted every WalletSolidity method exists on WalletApi, which the compiler enforces now that the handlers delegate by name. It would also fail on a legitimate WalletSolidity-only method implemented in place. GrpcInterceptorProbeTest kept an ordering assertion whose premise no longer holds — service-level attachment made registration order irrelevant to the cursor services — plus two library-property tests already covered through the production interceptor, and printed traces left over from probing. What that last class did earn is kept as CursorInterceptorServerTest: a real server proving gRPC runs the handler inline from onHalfClose on the same thread, which CursorInterceptorScopeTest cannot show on its synthetic harness and which fails silently in production if it breaks. Also drops two javadoc references to the deleted subset test.
1 parent 8e38bb9 commit 8b58c55

6 files changed

Lines changed: 128 additions & 626 deletions

File tree

‎framework/src/main/java/org/tron/core/services/RpcApiService.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -365,8 +365,8 @@ public void getDynamicProperties(EmptyMessage request,
365365

366366
/**
367367
* WalletSolidityApi is the full implementation of the {@code protocol.WalletSolidity} gRPC
368-
* service. Every method here is read-only and also present on {@link WalletApi}: this is a
369-
* read-only subset of {@code WalletApi}, guarded by {@code WalletSolidityApiMethodSubsetTest}.
368+
* service. Every method here is read-only and also present on {@link WalletApi}, so each one
369+
* delegates to the shared {@code WalletApi} singleton instead of repeating its body.
370370
*/
371371
public class WalletSolidityApi extends WalletSolidityImplBase {
372372

@@ -667,8 +667,8 @@ private TransactionListExtention transactionList2Extention(TransactionList trans
667667

668668
/**
669669
* WalletApi is the full implementation of the {@code protocol.Wallet} gRPC service, including
670-
* write and build endpoints. {@link WalletSolidityApi} is the read-only subset of this surface,
671-
* pinned by {@code WalletSolidityApiMethodSubsetTest}.
670+
* write and build endpoints. {@link WalletSolidityApi} is the read-only subset of this surface
671+
* and delegates its handlers here.
672672
*/
673673
public class WalletApi extends WalletImplBase {
674674

‎framework/src/test/java/org/tron/core/services/WalletSolidityApiMethodSubsetTest.java‎

Lines changed: 0 additions & 47 deletions
This file was deleted.

‎framework/src/test/java/org/tron/core/services/filter/CursorInterceptorAttachmentTest.java‎

Lines changed: 0 additions & 195 deletions
This file was deleted.
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
package org.tron.core.services.filter;
2+
3+
import static org.mockito.ArgumentMatchers.any;
4+
import static org.mockito.Mockito.doAnswer;
5+
import static org.mockito.Mockito.mock;
6+
import static org.mockito.Mockito.verify;
7+
8+
import io.grpc.ManagedChannel;
9+
import io.grpc.ManagedChannelBuilder;
10+
import io.grpc.Server;
11+
import io.grpc.ServerBuilder;
12+
import io.grpc.stub.StreamObserver;
13+
import java.lang.reflect.Field;
14+
import java.net.ServerSocket;
15+
import java.util.List;
16+
import java.util.concurrent.CopyOnWriteArrayList;
17+
import java.util.concurrent.ExecutorService;
18+
import java.util.concurrent.Executors;
19+
import java.util.concurrent.TimeUnit;
20+
import org.junit.After;
21+
import org.junit.Assert;
22+
import org.junit.Before;
23+
import org.junit.Test;
24+
import org.tron.api.DatabaseGrpc;
25+
import org.tron.api.DatabaseGrpc.DatabaseImplBase;
26+
import org.tron.api.GrpcAPI.EmptyMessage;
27+
import org.tron.core.db.Manager;
28+
import org.tron.core.db2.core.Chainbase;
29+
import org.tron.protos.Protocol.Block;
30+
31+
/**
32+
* Drives the production interceptor through a real gRPC server, which is the only place the
33+
* assumption it rests on can be checked: that gRPC runs the handler inline from
34+
* {@code onHalfClose}, on the same thread. The cursor is a {@link ThreadLocal}, so if that stops
35+
* holding the cursor never reaches the read path and the port serves HEAD data with no error —
36+
* responses stay well-formed, so nothing else notices.
37+
*
38+
* <p>CursorInterceptorScopeTest covers the interceptor's own logic on a synthetic harness; this is
39+
* the end-to-end half.
40+
*/
41+
public class CursorInterceptorServerTest {
42+
43+
private ExecutorService executor;
44+
45+
@Before
46+
public void setUp() {
47+
// a fixed thread pool mirrors the production server configuration
48+
executor = Executors.newFixedThreadPool(2, r -> {
49+
Thread thread = new Thread(r);
50+
thread.setName("cursor-rpc-executor-" + thread.getId());
51+
return thread;
52+
});
53+
}
54+
55+
@After
56+
public void tearDown() throws Exception {
57+
if (executor != null) {
58+
executor.shutdown();
59+
if (!executor.awaitTermination(5, TimeUnit.SECONDS)) {
60+
executor.shutdownNow();
61+
}
62+
}
63+
}
64+
65+
@Test
66+
public void testCursorIsSetAndRestoredOnTheHandlerThread() throws Exception {
67+
final List<String> setOn = new CopyOnWriteArrayList<>();
68+
final List<String> resetOn = new CopyOnWriteArrayList<>();
69+
final String[] handlerOn = new String[1];
70+
71+
Manager manager = mock(Manager.class);
72+
doAnswer(inv -> setOn.add(Thread.currentThread().getName()))
73+
.when(manager).setCursor(any(Chainbase.Cursor.class));
74+
doAnswer(inv -> resetOn.add(Thread.currentThread().getName()))
75+
.when(manager).resetCursor();
76+
77+
SolidityCursorInterceptor interceptor = new SolidityCursorInterceptor();
78+
Field dbManager = CursorServerInterceptor.class.getDeclaredField("dbManager");
79+
dbManager.setAccessible(true);
80+
dbManager.set(interceptor, manager);
81+
82+
int port = freePort();
83+
Server server = ServerBuilder.forPort(port)
84+
.executor(executor)
85+
.addService(new DatabaseImplBase() {
86+
@Override
87+
public void getNowBlock(EmptyMessage request, StreamObserver<Block> observer) {
88+
handlerOn[0] = Thread.currentThread().getName();
89+
observer.onNext(Block.getDefaultInstance());
90+
observer.onCompleted();
91+
}
92+
})
93+
.intercept(interceptor)
94+
.build()
95+
.start();
96+
97+
ManagedChannel channel = ManagedChannelBuilder.forAddress("127.0.0.1", port)
98+
.usePlaintext().directExecutor().build();
99+
try {
100+
DatabaseGrpc.newBlockingStub(channel).getNowBlock(EmptyMessage.getDefaultInstance());
101+
102+
Assert.assertEquals("cursor must be set exactly once per call", 1, setOn.size());
103+
Assert.assertEquals("cursor must be restored exactly once per call", 1, resetOn.size());
104+
Assert.assertNotNull("handler did not run", handlerOn[0]);
105+
// the ThreadLocal cursor only reaches the read path if it is set on the handler's thread
106+
Assert.assertEquals("cursor was set on a thread other than the handler's",
107+
handlerOn[0], setOn.get(0));
108+
Assert.assertEquals("cursor was restored on a thread other than the handler's",
109+
handlerOn[0], resetOn.get(0));
110+
verify(manager).setCursor(Chainbase.Cursor.SOLIDITY);
111+
} finally {
112+
channel.shutdownNow();
113+
server.shutdownNow();
114+
server.awaitTermination(5, TimeUnit.SECONDS);
115+
}
116+
}
117+
118+
private static int freePort() throws Exception {
119+
try (ServerSocket socket = new ServerSocket(0)) {
120+
return socket.getLocalPort();
121+
}
122+
}
123+
}

‎framework/src/test/java/org/tron/core/services/filter/CursorInterceptorWiringTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
* <p>Registering without the interceptor binds the {@code addService(BindableService)} overload
3636
* rather than the {@code addService(ServerServiceDefinition)} one, so the two are distinguishable
3737
* here. What the interceptor does once attached is pinned by CursorInterceptorScopeTest and
38-
* GrpcInterceptorProbeTest.
38+
* CursorInterceptorServerTest.
3939
*/
4040
public class CursorInterceptorWiringTest {
4141

0 commit comments

Comments
 (0)