Skip to content

Commit e8d30a7

Browse files
acogoluegnesacogoluegnes
committed
Set up Spotbugs and squash warnings
1 parent 0711072 commit e8d30a7

18 files changed

+79
-54
lines changed

‎pom.xml

+28
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@
8989
<spotless.version>2.43.0</spotless.version>
9090
<google-java-format.version>1.22.0</google-java-format.version>
9191
<jacoco.version>0.8.12</jacoco.version>
92+
<spotbugs-maven-plugin.version>4.8.5.0</spotbugs-maven-plugin.version>
93+
<spotbugs.version>4.8.5</spotbugs.version>
9294
<!-- for documentation -->
9395
<broker.version>3.12</broker.version>
9496
<!-- to sign artifacts when releasing -->
@@ -302,6 +304,12 @@
302304
<scope>test</scope>
303305
</dependency>
304306

307+
<dependency>
308+
<groupId>com.github.spotbugs</groupId>
309+
<artifactId>spotbugs-annotations</artifactId>
310+
<version>4.8.5</version>
311+
<scope>provided</scope>
312+
</dependency>
305313

306314
</dependencies>
307315

@@ -566,6 +574,26 @@
566574
</executions>
567575
</plugin>
568576

577+
<plugin>
578+
<groupId>com.github.spotbugs</groupId>
579+
<artifactId>spotbugs-maven-plugin</artifactId>
580+
<version>${spotbugs-maven-plugin.version}</version>
581+
<dependencies>
582+
<dependency>
583+
<groupId>com.github.spotbugs</groupId>
584+
<artifactId>spotbugs</artifactId>
585+
<version>${spotbugs.version}</version>
586+
</dependency>
587+
</dependencies>
588+
<executions>
589+
<execution>
590+
<goals>
591+
<goal>check</goal>
592+
</goals>
593+
</execution>
594+
</executions>
595+
</plugin>
596+
569597
</plugins>
570598
<extensions>
571599
<extension>

‎src/main/java/com/rabbitmq/stream/BackOffDelayPolicy.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public String toString() {
9898
}
9999
}
100100

101-
class FixedWithInitialDelayAndTimeoutBackOffPolicy implements BackOffDelayPolicy {
101+
final class FixedWithInitialDelayAndTimeoutBackOffPolicy implements BackOffDelayPolicy {
102102

103103
private final int attemptLimitBeforeTimeout;
104104
private final BackOffDelayPolicy delegate;

‎src/main/java/com/rabbitmq/stream/ByteCapacity.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public long toBytes() {
105105
public static ByteCapacity from(String value) {
106106
Matcher matcher = PATTERN.matcher(value);
107107
if (matcher.matches()) {
108-
long size = Long.valueOf(matcher.group(GROUP_SIZE));
108+
long size = Long.parseLong(matcher.group(GROUP_SIZE));
109109
String unit = matcher.group(GROUP_UNIT);
110110
ByteCapacity result;
111111
if (unit == null) {

‎src/main/java/com/rabbitmq/stream/Codec.java

+4
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
// [email protected].
1515
package com.rabbitmq.stream;
1616

17+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
18+
1719
/**
1820
* Codec to encode and decode messages.
1921
*
@@ -34,11 +36,13 @@ class EncodedMessage {
3436
private final int size;
3537
private final byte[] data;
3638

39+
@SuppressFBWarnings("EI_EXPOSE_REP2")
3740
public EncodedMessage(int size, byte[] data) {
3841
this.size = size;
3942
this.data = data;
4043
}
4144

45+
@SuppressFBWarnings("EI_EXPOSE_REP")
4246
public byte[] getData() {
4347
return data;
4448
}

‎src/main/java/com/rabbitmq/stream/codec/SwiftMqCodec.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ protected static AMQPType convertToSwiftMqType(Object value) {
320320
return new AMQPSymbol(value.toString());
321321
} else if (value instanceof UUID) {
322322
return new AMQPUuid((UUID) value);
323-
} else if (value == value) {
323+
} else if (value == null) {
324324
return AMQPNull.NULL;
325325
} else {
326326
throw new IllegalArgumentException(

‎src/main/java/com/rabbitmq/stream/impl/Client.java

+12-5
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
import com.rabbitmq.stream.sasl.SaslConfiguration;
5656
import com.rabbitmq.stream.sasl.SaslMechanism;
5757
import com.rabbitmq.stream.sasl.UsernamePasswordCredentialsProvider;
58+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
5859
import io.netty.bootstrap.Bootstrap;
5960
import io.netty.buffer.ByteBuf;
6061
import io.netty.buffer.ByteBufAllocator;
@@ -194,10 +195,12 @@ public long applyAsLong(Object value) {
194195
private final boolean filteringSupported;
195196
private final Runnable superStreamManagementCommandVersionsCheck;
196197

198+
@SuppressFBWarnings("CT_CONSTRUCTOR_THROW")
197199
public Client() {
198200
this(new ClientParameters());
199201
}
200202

203+
@SuppressFBWarnings("CT_CONSTRUCTOR_THROW")
201204
public Client(ClientParameters parameters) {
202205
this.publishConfirmListener = parameters.publishConfirmListener;
203206
this.publishErrorListener = parameters.publishErrorListener;
@@ -1678,7 +1681,7 @@ String serverAdvertisedHost() {
16781681
}
16791682

16801683
int serverAdvertisedPort() {
1681-
return Integer.valueOf(this.connectionProperties("advertised_port"));
1684+
return Integer.parseInt(this.connectionProperties("advertised_port"));
16821685
}
16831686

16841687
public String brokerVersion() {
@@ -2233,7 +2236,7 @@ public StreamMetadata(String stream, short responseCode, Broker leader, List<Bro
22332236
this.stream = stream;
22342237
this.responseCode = responseCode;
22352238
this.leader = leader;
2236-
this.replicas = replicas;
2239+
this.replicas = replicas == null ? null : Collections.unmodifiableList(replicas);
22372240
}
22382241

22392242
public short getResponseCode() {
@@ -2679,12 +2682,16 @@ public StreamParametersBuilder maxLengthTb(long teraBytes) {
26792682
}
26802683

26812684
public StreamParametersBuilder maxSegmentSizeBytes(long bytes) {
2682-
this.parameters.put("stream-max-segment-size-bytes", String.valueOf(bytes));
2685+
if (bytes <= 0) {
2686+
this.parameters.remove("stream-max-segment-size-bytes");
2687+
} else {
2688+
this.parameters.put("stream-max-segment-size-bytes", String.valueOf(bytes));
2689+
}
26832690
return this;
26842691
}
26852692

26862693
public StreamParametersBuilder maxSegmentSizeBytes(ByteCapacity bytes) {
2687-
return maxSegmentSizeBytes(bytes.toBytes());
2694+
return maxSegmentSizeBytes(bytes == null ? 0L : bytes.toBytes());
26882695
}
26892696

26902697
public StreamParametersBuilder maxSegmentSizeKb(long kiloBytes) {
@@ -2733,7 +2740,7 @@ public StreamParametersBuilder put(String key, String value) {
27332740
}
27342741

27352742
public Map<String, String> build() {
2736-
return parameters;
2743+
return new HashMap<>(parameters);
27372744
}
27382745
}
27392746

‎src/main/java/com/rabbitmq/stream/impl/ConsumersCoordinator.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ class ConsumersCoordinator {
5757

5858
static final int MAX_SUBSCRIPTIONS_PER_CLIENT = 256;
5959
static final int MAX_ATTEMPT_BEFORE_FALLING_BACK_TO_LEADER = 5;
60+
private static final boolean DEBUG = false;
6061

6162
static final OffsetSpecification DEFAULT_OFFSET_SPECIFICATION = OffsetSpecification.next();
6263

@@ -70,7 +71,6 @@ class ConsumersCoordinator {
7071
private final NavigableSet<ClientSubscriptionsManager> managers = new ConcurrentSkipListSet<>();
7172
private final AtomicLong trackerIdSequence = new AtomicLong(0);
7273

73-
private final boolean debug = false;
7474
private final List<SubscriptionTracker> trackers = new CopyOnWriteArrayList<>();
7575
private final ExecutorServiceFactory executorServiceFactory =
7676
new DefaultExecutorServiceFactory(
@@ -144,7 +144,7 @@ Runnable subscribe(
144144
throw new StreamException(e.getMessage());
145145
}
146146

147-
if (debug) {
147+
if (DEBUG) {
148148
this.trackers.add(subscriptionTracker);
149149
return () -> {
150150
try {
@@ -366,7 +366,7 @@ public String toString() {
366366
})
367367
.collect(Collectors.joining(",")));
368368
builder.append("]");
369-
if (debug) {
369+
if (DEBUG) {
370370
builder.append(",");
371371
builder.append("\"subscription_count\" : ").append(this.trackers.size()).append(",");
372372
builder.append("\"subscriptions\" : [");
@@ -1168,7 +1168,7 @@ synchronized void close() {
11681168
SubscriptionTracker tracker = this.subscriptionTrackers.get(i);
11691169
if (tracker != null) {
11701170
try {
1171-
if (this.client != null && this.client.isOpen() && tracker.consumer.isOpen()) {
1171+
if (this.client.isOpen() && tracker.consumer.isOpen()) {
11721172
this.client.unsubscribe(tracker.subscriptionIdInClient);
11731173
}
11741174
} catch (Exception e) {
@@ -1184,7 +1184,7 @@ synchronized void close() {
11841184

11851185
streamToStreamSubscriptions.clear();
11861186

1187-
if (this.client != null && this.client.isOpen()) {
1187+
if (this.client.isOpen()) {
11881188
this.client.close();
11891189
}
11901190
}

‎src/main/java/com/rabbitmq/stream/impl/JdkChunkChecksum.java

+2-4
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public void checksum(ByteBuf byteBuf, long dataLength, long expected) {
7979
}
8080
}
8181

82-
private static class ByteBufferDirectByteBufChecksum implements ChunkChecksum {
82+
private static final class ByteBufferDirectByteBufChecksum implements ChunkChecksum {
8383

8484
private final Supplier<Checksum> checksumSupplier;
8585
private final Method updateMethod;
@@ -105,9 +105,7 @@ public void checksum(ByteBuf byteBuf, long dataLength, long expected) {
105105
try {
106106
this.updateMethod.invoke(
107107
checksum, byteBuf.nioBuffer(byteBuf.readerIndex(), byteBuf.readableBytes()));
108-
} catch (IllegalAccessException e) {
109-
throw new StreamException("Error while calculating CRC", e);
110-
} catch (InvocationTargetException e) {
108+
} catch (IllegalAccessException | InvocationTargetException e) {
111109
throw new StreamException("Error while calculating CRC", e);
112110
}
113111
}

‎src/main/java/com/rabbitmq/stream/impl/MessageBatch.java

+2-24
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import com.rabbitmq.stream.Message;
1818
import com.rabbitmq.stream.compression.Compression;
19+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
1920
import java.util.ArrayList;
2021
import java.util.List;
2122

@@ -24,18 +25,11 @@ public final class MessageBatch {
2425
final Compression compression;
2526
final List<Message> messages;
2627

27-
public MessageBatch() {
28-
this(Compression.NONE, new ArrayList<>());
29-
}
30-
3128
public MessageBatch(Compression compression) {
3229
this(compression, new ArrayList<>());
3330
}
3431

35-
public MessageBatch(List<Message> messages) {
36-
this(Compression.NONE, messages);
37-
}
38-
32+
@SuppressFBWarnings("EI_EXPOSE_REP2")
3933
public MessageBatch(Compression compression, List<Message> messages) {
4034
this.compression = compression;
4135
this.messages = messages;
@@ -45,20 +39,4 @@ public MessageBatch add(Message message) {
4539
this.messages.add(message);
4640
return this;
4741
}
48-
49-
public List<Message> getMessages() {
50-
return messages;
51-
}
52-
53-
/*
54-
0 = no compression
55-
1 = gzip
56-
2 = snappy
57-
3 = lz4
58-
4 = zstd
59-
5 = reserved
60-
6 = reserved
61-
7 = user
62-
*/
63-
6442
}

‎src/main/java/com/rabbitmq/stream/impl/ParameterizedTypeReference.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
// [email protected].
1515
package com.rabbitmq.stream.impl;
1616

17+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
1718
import java.lang.reflect.ParameterizedType;
1819
import java.lang.reflect.Type;
1920

@@ -26,10 +27,11 @@
2627
*
2728
* @param <T>
2829
*/
29-
public abstract class ParameterizedTypeReference<T> {
30+
abstract class ParameterizedTypeReference<T> {
3031

3132
private final Type type;
3233

34+
@SuppressFBWarnings("CT_CONSTRUCTOR_THROW")
3335
protected ParameterizedTypeReference() {
3436
Class<?> parameterizedTypeReferenceSubclass =
3537
findParameterizedTypeReferenceSubclass(getClass());

‎src/main/java/com/rabbitmq/stream/impl/ProducersCoordinator.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ class ProducersCoordinator {
5959

6060
static final int MAX_PRODUCERS_PER_CLIENT = 256;
6161
static final int MAX_TRACKING_CONSUMERS_PER_CLIENT = 50;
62+
private static final boolean DEBUG = false;
6263
private static final Logger LOGGER = LoggerFactory.getLogger(ProducersCoordinator.class);
6364
private final StreamEnvironment environment;
6465
private final ClientFactory clientFactory;
@@ -67,7 +68,6 @@ class ProducersCoordinator {
6768
private final AtomicLong managerIdSequence = new AtomicLong(0);
6869
private final NavigableSet<ClientProducersManager> managers = new ConcurrentSkipListSet<>();
6970
private final AtomicLong trackerIdSequence = new AtomicLong(0);
70-
private final boolean debug = false;
7171
private final List<ProducerTracker> producerTrackers = new CopyOnWriteArrayList<>();
7272
private final ExecutorServiceFactory executorServiceFactory =
7373
new DefaultExecutorServiceFactory(
@@ -97,7 +97,7 @@ Runnable registerProducer(StreamProducer producer, String reference, String stre
9797
() -> {
9898
ProducerTracker tracker =
9999
new ProducerTracker(trackerIdSequence.getAndIncrement(), reference, stream, producer);
100-
if (debug) {
100+
if (DEBUG) {
101101
this.producerTrackers.add(tracker);
102102
}
103103
return registerAgentTracker(tracker, stream);
@@ -119,7 +119,7 @@ private Runnable registerAgentTracker(AgentTracker tracker, String stream) {
119119

120120
addToManager(broker, tracker);
121121

122-
if (debug) {
122+
if (DEBUG) {
123123
return () -> {
124124
if (tracker instanceof ProducerTracker) {
125125
try {
@@ -275,7 +275,7 @@ public String toString() {
275275
"tracking_consumer_count",
276276
this.managers.stream().mapToInt(m -> m.trackingConsumerTrackers.size()).sum()))
277277
.append(",");
278-
if (debug) {
278+
if (DEBUG) {
279279
builder.append(jsonField("producer_tracker_count", this.producerTrackers.size())).append(",");
280280
}
281281
builder.append(quote("clients")).append(" : [");
@@ -321,7 +321,7 @@ public String toString() {
321321
})
322322
.collect(Collectors.joining(",")));
323323
builder.append("]");
324-
if (debug) {
324+
if (DEBUG) {
325325
builder.append(",");
326326
builder.append("\"producer_trackers\" : [");
327327
builder.append(

‎src/main/java/com/rabbitmq/stream/impl/StreamConsumer.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.rabbitmq.stream.impl.StreamConsumerBuilder.TrackingConfiguration;
2626
import com.rabbitmq.stream.impl.StreamEnvironment.TrackingConsumerRegistration;
2727
import com.rabbitmq.stream.impl.Utils.CompositeConsumerUpdateListener;
28+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
2829
import java.util.Collections;
2930
import java.util.Map;
3031
import java.util.Objects;
@@ -64,6 +65,7 @@ class StreamConsumer implements Consumer {
6465
private final boolean sac;
6566
private final OffsetSpecification initialOffsetSpecification;
6667

68+
@SuppressFBWarnings("CT_CONSTRUCTOR_THROW")
6769
StreamConsumer(
6870
String stream,
6971
OffsetSpecification offsetSpecification,
@@ -188,7 +190,8 @@ class StreamConsumer implements Consumer {
188190
return result;
189191
};
190192
// just a trick for testing
191-
if (consumerUpdateListener instanceof CompositeConsumerUpdateListener) {
193+
// we know the update listener is either null or a composite one
194+
if (consumerUpdateListener != null) {
192195
((CompositeConsumerUpdateListener) consumerUpdateListener).add(defaultListener);
193196
this.consumerUpdateListener = consumerUpdateListener;
194197
} else {

0 commit comments

Comments
 (0)