You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by sh...@apache.org on 2022/09/10 10:51:57 UTC
[pulsar] branch master updated: [improve][misc] reduce WhiteBox usage (#17578)
This is an automated email from the ASF dual-hosted git repository.
shoothzj pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/master by this push:
new 06c14cbeb21 [improve][misc] reduce WhiteBox usage (#17578)
06c14cbeb21 is described below
commit 06c14cbeb21ff53e52d1e674fec46cc07b558ad2
Author: tison <wa...@gmail.com>
AuthorDate: Sat Sep 10 18:51:48 2022 +0800
[improve][misc] reduce WhiteBox usage (#17578)
Master Issue: #16912
Remove `WhiteBox` usage in `pulsar-client` and `pulsar-client-tools`. We then have only usages in `pulsar-broker`, `pulsar-io-elastic-search`, and `testmocks`.
cc @nicoloboschi @lhotari @Shoothzj @eolivelli
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc-required`
(Your PR needs to update docs and you will update later)
- [x] `doc-not-needed`
(Please explain why)
- [ ] `doc`
(Your PR contains doc changes)
- [ ] `doc-complete`
(Docs have been already added)
---
.../apache/pulsar/client/cli/PulsarClientTool.java | 4 +-
.../org/apache/pulsar/shell/PulsarShellTest.java | 15 ++---
.../pulsar/client/impl/PulsarClientImpl.java | 5 ++
.../pulsar/client/impl/PulsarClientImplTest.java | 74 ++++++++++------------
4 files changed, 46 insertions(+), 52 deletions(-)
diff --git a/pulsar-client-tools/src/main/java/org/apache/pulsar/client/cli/PulsarClientTool.java b/pulsar-client-tools/src/main/java/org/apache/pulsar/client/cli/PulsarClientTool.java
index 648505b7d75..5849cf8415f 100644
--- a/pulsar-client-tools/src/main/java/org/apache/pulsar/client/cli/PulsarClientTool.java
+++ b/pulsar-client-tools/src/main/java/org/apache/pulsar/client/cli/PulsarClientTool.java
@@ -96,8 +96,8 @@ public class PulsarClientTool {
protected JCommander jcommander;
IUsageFormatter usageFormatter;
- CmdProduce produceCommand;
- CmdConsume consumeCommand;
+ protected CmdProduce produceCommand;
+ protected CmdConsume consumeCommand;
CmdGenerateDocumentation generateDocumentation;
public PulsarClientTool(Properties properties) {
diff --git a/pulsar-client-tools/src/test/java/org/apache/pulsar/shell/PulsarShellTest.java b/pulsar-client-tools/src/test/java/org/apache/pulsar/shell/PulsarShellTest.java
index 1102845ada2..2bbef0031bb 100644
--- a/pulsar-client-tools/src/test/java/org/apache/pulsar/shell/PulsarShellTest.java
+++ b/pulsar-client-tools/src/test/java/org/apache/pulsar/shell/PulsarShellTest.java
@@ -45,7 +45,6 @@ import org.jline.reader.UserInterruptException;
import org.jline.reader.impl.LineReaderImpl;
import org.jline.terminal.Terminal;
import org.jline.terminal.TerminalBuilder;
-import org.powermock.reflect.Whitebox;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.testng.annotations.BeforeMethod;
@@ -110,11 +109,11 @@ public class PulsarShellTest {
@Override
protected ClientShell createClientShell(Properties properties) {
- final ClientShell clientShell = new ClientShell(properties);
final CmdProduce cmdProduce = mock(CmdProduce.class);
cmdProduceHolder.set(cmdProduce);
- Whitebox.setInternalState(clientShell, "produceCommand", cmdProduceHolder.get());
- return clientShell;
+ return new ClientShell(properties) {{
+ this.produceCommand = cmdProduce;
+ }};
}
@Override
@@ -127,7 +126,7 @@ public class PulsarShellTest {
}
private static class SystemExitCalledException extends RuntimeException {
- private int code;
+ private final int code;
public SystemExitCalledException(int code) {
this.code = code;
@@ -145,7 +144,7 @@ public class PulsarShellTest {
@Test
- public void testInteractiveMode() throws Exception{
+ public void testInteractiveMode() throws Exception {
Terminal terminal = TerminalBuilder.builder().build();
final MockLineReader linereader = new MockLineReader(terminal);
@@ -163,7 +162,7 @@ public class PulsarShellTest {
}
@Test
- public void testFileMode() throws Exception{
+ public void testFileMode() throws Exception {
Terminal terminal = TerminalBuilder.builder().build();
final MockLineReader linereader = new MockLineReader(terminal);
final Properties props = new Properties();
@@ -194,7 +193,7 @@ public class PulsarShellTest {
try {
testPulsarShell.run((a) -> linereader, (a) -> terminal);
fail();
- } catch (SystemExitCalledException ex) {
+ } catch (SystemExitCalledException ex) {
assertEquals(ex.code, 1);
}
diff --git a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java
index c436f129789..3924e5fbc22 100644
--- a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java
+++ b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java
@@ -993,6 +993,11 @@ public class PulsarClientImpl implements PulsarClient {
return eventLoopGroup;
}
+ @VisibleForTesting
+ void setLookup(LookupService lookup) {
+ this.lookup = lookup;
+ }
+
public LookupService getLookup() {
return lookup;
}
diff --git a/pulsar-client/src/test/java/org/apache/pulsar/client/impl/PulsarClientImplTest.java b/pulsar-client/src/test/java/org/apache/pulsar/client/impl/PulsarClientImplTest.java
index 93fe3e34876..5c7228919ab 100644
--- a/pulsar-client/src/test/java/org/apache/pulsar/client/impl/PulsarClientImplTest.java
+++ b/pulsar-client/src/test/java/org/apache/pulsar/client/impl/PulsarClientImplTest.java
@@ -22,15 +22,14 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.nullable;
import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNotSame;
import static org.testng.Assert.assertSame;
import static org.testng.Assert.assertThrows;
import static org.testng.Assert.assertTrue;
-
import io.netty.buffer.ByteBuf;
import io.netty.channel.Channel;
import io.netty.channel.ChannelFuture;
@@ -39,7 +38,6 @@ import io.netty.channel.ChannelPromise;
import io.netty.channel.EventLoopGroup;
import io.netty.util.HashedWheelTimer;
import io.netty.util.concurrent.DefaultThreadFactory;
-
import java.lang.reflect.Field;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
@@ -49,7 +47,6 @@ import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ThreadFactory;
import java.util.regex.Pattern;
-
import lombok.Cleanup;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.pulsar.client.api.PulsarClientException;
@@ -64,26 +61,15 @@ import org.apache.pulsar.common.naming.TopicName;
import org.apache.pulsar.common.partition.PartitionedTopicMetadata;
import org.apache.pulsar.common.util.netty.EventLoopUtil;
import org.mockito.Mockito;
-import org.powermock.reflect.Whitebox;
import org.testng.annotations.AfterMethod;
-import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
/**
* PulsarClientImpl unit tests.
*/
public class PulsarClientImplTest {
- private PulsarClientImpl clientImpl;
private EventLoopGroup eventLoopGroup;
- @BeforeMethod(alwaysRun = true)
- public void setup() throws PulsarClientException {
- ClientConfigurationData conf = new ClientConfigurationData();
- conf.setServiceUrl("pulsar://localhost:6650");
- initializeEventLoopGroup(conf);
- clientImpl = new PulsarClientImpl(conf, eventLoopGroup);
- }
-
private void initializeEventLoopGroup(ClientConfigurationData conf) {
ThreadFactory threadFactory = new DefaultThreadFactory("client-test-stats", Thread.currentThread().isDaemon());
eventLoopGroup = EventLoopUtil.newEventLoopGroup(conf.getNumIoThreads(), false, threadFactory);
@@ -91,10 +77,6 @@ public class PulsarClientImplTest {
@AfterMethod(alwaysRun = true)
public void teardown() throws Exception {
- if (clientImpl != null) {
- clientImpl.close();
- clientImpl = null;
- }
if (eventLoopGroup != null) {
eventLoopGroup.shutdownGracefully().get();
eventLoopGroup = null;
@@ -103,9 +85,13 @@ public class PulsarClientImplTest {
@Test
public void testIsClosed() throws Exception {
- assertFalse(clientImpl.isClosed());
- clientImpl.close();
- assertTrue(clientImpl.isClosed());
+ ClientConfigurationData conf = new ClientConfigurationData();
+ conf.setServiceUrl("pulsar://localhost:6650");
+ initializeEventLoopGroup(conf);
+ PulsarClientImpl client = new PulsarClientImpl(conf, eventLoopGroup);
+ assertFalse(client.isClosed());
+ client.close();
+ assertTrue(client.isClosed());
}
@Test
@@ -138,8 +124,11 @@ public class PulsarClientImplTest {
.thenReturn(CompletableFuture.completedFuture(mock(ProducerResponse.class)));
when(pool.getConnection(any(InetSocketAddress.class), any(InetSocketAddress.class)))
.thenReturn(CompletableFuture.completedFuture(cnx));
- Whitebox.setInternalState(clientImpl, "cnxPool", pool);
- Whitebox.setInternalState(clientImpl, "lookup", lookup);
+
+ ClientConfigurationData conf = new ClientConfigurationData();
+ conf.setServiceUrl("pulsar://localhost:6650");
+ PulsarClientImpl client = new PulsarClientImpl(conf, eventLoopGroup, pool);
+ client.setLookup(lookup);
List<ConsumerBase<byte[]>> consumers = new ArrayList<>();
/**
@@ -148,24 +137,24 @@ public class PulsarClientImplTest {
ConsumerConfigurationData<byte[]> consumerConf0 = new ConsumerConfigurationData<>();
consumerConf0.setSubscriptionName("test-subscription0");
consumerConf0.setTopicsPattern(Pattern.compile("test-topic"));
- consumers.add((ConsumerBase) clientImpl.subscribeAsync(consumerConf0).get());
+ consumers.add((ConsumerBase<byte[]>) client.subscribeAsync(consumerConf0).get());
/**
* {@link org.apache.pulsar.client.impl.PulsarClientImpl#singleTopicSubscribeAsync}
*/
ConsumerConfigurationData<byte[]> consumerConf1 = new ConsumerConfigurationData<>();
consumerConf1.setSubscriptionName("test-subscription1");
consumerConf1.setTopicNames(Collections.singleton("test-topic"));
- consumers.add((ConsumerBase) clientImpl.subscribeAsync(consumerConf1).get());
+ consumers.add((ConsumerBase<byte[]>) client.subscribeAsync(consumerConf1).get());
/**
* {@link org.apache.pulsar.client.impl.PulsarClientImpl#multiTopicSubscribeAsync}
*/
ConsumerConfigurationData<byte[]> consumerConf2 = new ConsumerConfigurationData<>();
consumerConf2.setSubscriptionName("test-subscription2");
- consumers.add((ConsumerBase) clientImpl.subscribeAsync(consumerConf2).get());
+ consumers.add((ConsumerBase<byte[]>) client.subscribeAsync(consumerConf2).get());
consumers.forEach(consumer ->
assertNotSame(consumer.getState(), HandlerState.State.Closed));
- clientImpl.close();
+ client.close();
consumers.forEach(consumer ->
assertSame(consumer.getState(), HandlerState.State.Closed));
}
@@ -199,27 +188,22 @@ public class PulsarClientImplTest {
client.timer().stop();
}
- @Test(expectedExceptions = PulsarClientException.class)
+ @Test(expectedExceptions = PulsarClientException.InvalidConfigurationException.class)
public void testNewTransactionWhenDisable() throws Exception {
ClientConfigurationData conf = new ClientConfigurationData();
conf.setServiceUrl("pulsar://localhost:6650");
conf.setEnableTransaction(false);
- PulsarClientImpl pulsarClient = null;
- try {
- pulsarClient = new PulsarClientImpl(conf);
- } catch (PulsarClientException e) {
- e.printStackTrace();
+ try (PulsarClientImpl client = new PulsarClientImpl(conf)) {
+ client.newTransaction();
}
- pulsarClient.newTransaction();
}
@Test
- public void testResourceCleanup() throws PulsarClientException {
- ClientConfigurationData conf = clientImpl.conf;
+ public void testResourceCleanup() throws Exception {
+ ClientConfigurationData conf = new ClientConfigurationData();
conf.setServiceUrl("");
initializeEventLoopGroup(conf);
- ConnectionPool connectionPool = new ConnectionPool(conf, eventLoopGroup);
- try {
+ try (ConnectionPool connectionPool = new ConnectionPool(conf, eventLoopGroup)) {
assertThrows(() -> new PulsarClientImpl(conf, eventLoopGroup, connectionPool));
} finally {
// Externally passed eventLoopGroup should not be shutdown.
@@ -229,7 +213,10 @@ public class PulsarClientImplTest {
@Test
public void testInitializingWithExecutorProviders() throws PulsarClientException {
- ClientConfigurationData conf = clientImpl.conf;
+ ClientConfigurationData conf = new ClientConfigurationData();
+ conf.setServiceUrl("pulsar://localhost:6650");
+ initializeEventLoopGroup(conf);
+
@Cleanup("shutdownNow")
ExecutorProvider executorProvider = new ExecutorProvider(2, "shared-executor");
@Cleanup("shutdownNow")
@@ -256,11 +243,14 @@ public class PulsarClientImplTest {
expectedExceptionsMessageRegExp = "Both externalExecutorProvider and internalExecutorProvider must be " +
"specified or unspecified.")
public void testBothExecutorProvidersMustBeSpecified() throws PulsarClientException {
- ClientConfigurationData conf = clientImpl.conf;
+ ClientConfigurationData conf = new ClientConfigurationData();
+ conf.setServiceUrl("pulsar://localhost:6650");
+ initializeEventLoopGroup(conf);
@Cleanup("shutdownNow")
ExecutorProvider executorProvider = new ExecutorProvider(2, "shared-executor");
@Cleanup
- PulsarClientImpl client2 = PulsarClientImpl.builder().conf(conf)
+ PulsarClientImpl ignore = PulsarClientImpl.builder()
+ .conf(conf)
.internalExecutorProvider(executorProvider)
.build();
}