You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by hs...@apache.org on 2021/04/16 20:45:40 UTC
[bookkeeper] branch master updated: ISSUE #2640: BP-43 Integrate
spotbugs plugin with gradle
This is an automated email from the ASF dual-hosted git repository.
hsaputra pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/master by this push:
new 44b1069 ISSUE #2640: BP-43 Integrate spotbugs plugin with gradle
44b1069 is described below
commit 44b1069e20ce6cc84302ed685936080f612bd8f0
Author: Prashant Kumar <65...@users.noreply.github.com>
AuthorDate: Fri Apr 16 13:45:31 2021 -0700
ISSUE #2640: BP-43 Integrate spotbugs plugin with gradle
### Motivation
**Migrate bookkeeper to gradle spotbug plugin.**
spotbugs is gradle as well as maven plugin which checks for obvious bugs in source code.
**How to run**
`./gradlew spotbugsMain `
### Changes
- Integrate gradle build with spotbugs plugin
- Second commit is for fixing issues reported by spotbugs plugin
### Test
Just to validate it's working.
I introduced following bug.
```
public void test() {
+
+ String[] field = {"a", "b", "c", "s", "e"};
+
+ //concatenates strings using + in a loop
+ String ip = "127.0.0.1";
+ String s = "";
+ for (int i = 0; i < field.length; ++i) {
+ s = s + field[i];
+ }
+
+ System.out.println(ip);
+
+ }
```
spotbugs plugin reported following violation.
```
<BugInstance type="SBSC_USE_STRINGBUFFER_CONCATENATION" priority="2" rank="18" abbrev="SBSC" category="PERFORMANCE" instanceHash="2aa8e4ebcc4cd86190fe3e7bd8a4913c" instanceOccurrenceNum="0" instanceOccurrenceMax="0">
<ShortMessage>Method concatenates strings using + in a loop</ShortMessage>
<LongMessage>org.apache.bookkeeper.meta.LongZkLedgerIdGenerator.test() concatenates strings using + in a loop</LongMessage>
<Class classname="org.apache.bookkeeper.meta.LongZkLedgerIdGenerator" primary="true">
<SourceLine classname="org.apache.bookkeeper.meta.LongZkLedgerIdGenerator" start="58" end="350" sourcefile="LongZkLedgerIdGenerator.java" sourcepath="org/apache/bookkeeper/meta/LongZkLedgerIdGenerator.java" relSourcepath="java/org/apache/bookkeeper/meta/LongZkLedgerIdGenerator.java">
<Message>At LongZkLedgerIdGenerator.java:[lines 58-350]</Message>
</SourceLine>
<Message>In class org.apache.bookkeeper.meta.LongZkLedgerIdGenerator</Message>
</Class>
```
Master Issue: #2640
Reviewers: Henry Saputra <hs...@apache.org>, Enrico Olivelli <eo...@gmail.com>
This closes #2682 from pkumar-singh/merge_internal_gradle_spotbugs, closes #2640 and squashes the following commits:
588373241 [Prashant] Fix for gradle spotbug plugin found violation
2bc79b067 [Prashant] ISSUE-2640: BP-43 Gradle migration integrating with spotbug gradle plugin
f1a1f1219 [Surinder Singh] staging and setting up vote for release candidates (#2681)
2dd4afecd [Prashant Kumar] ISSUE-2640: BP-43: Gradle integration with RAT plugin (#2683)
79768fee1 [Jack Vanlightly] ISSUE #2615: Fix for invalid ensemble issue during ledger recovery
646e59089 [Prashant Kumar] ISSUE #2640: BP-43 integrate gradle javadoc plugin
d70153f76 [hangc0276] Update documentation with default value for openLedgerRereplicationGracePeriod config option
---
.../codahale-metrics-provider/build.gradle | 1 +
build.gradle | 8 ++
.../main/resources/bookkeeper/findbugsExclude.xml | 160 +++++++++++++++++++++
gradle.properties | 1 +
settings.gradle | 1 +
.../statelib/impl/mvcc/MVCCStoreImpl.java | 5 +-
.../impl/metadata/stream/MetaRangeImpl.java | 2 +-
7 files changed, 175 insertions(+), 3 deletions(-)
diff --git a/bookkeeper-stats-providers/codahale-metrics-provider/build.gradle b/bookkeeper-stats-providers/codahale-metrics-provider/build.gradle
index b60c3c5..2ac1dbb 100644
--- a/bookkeeper-stats-providers/codahale-metrics-provider/build.gradle
+++ b/bookkeeper-stats-providers/codahale-metrics-provider/build.gradle
@@ -41,6 +41,7 @@ dependencies {
annotationProcessor depLibs.lombok
testAnnotationProcessor depLibs.lombok
+ testImplementation depLibs.junit
}
jar {
diff --git a/build.gradle b/build.gradle
index 97d148b..2cea442 100644
--- a/build.gradle
+++ b/build.gradle
@@ -28,6 +28,7 @@ plugins {
id "com.github.vlsi.stage-vote-release" version "1.73"
id 'checkstyle'
id 'org.nosphere.apache.rat'
+ id 'com.github.spotbugs'
}
subprojects {
@@ -68,6 +69,7 @@ allprojects {
apply plugin: 'signing'
apply plugin: 'org.nosphere.apache.rat'
apply plugin: "checkstyle"
+ apply plugin: 'com.github.spotbugs'
checkstyle {
toolVersion "${checkStyleVersion}"
configFile file("$rootDir/buildtools/src/main/resources/bookkeeper/checkstyle.xml")
@@ -79,6 +81,12 @@ allprojects {
source ='src/test/java'
}
}
+
+ spotbugs {
+ toolVersion = '3.1.8'
+ excludeFilter = file("$rootDir/buildtools/src/main/resources/bookkeeper/findbugsExclude.xml")
+ reportLevel = 'high'
+ }
task testJar(type: Jar, dependsOn: testClasses) {
classifier = 'tests'
diff --git a/buildtools/src/main/resources/bookkeeper/findbugsExclude.xml b/buildtools/src/main/resources/bookkeeper/findbugsExclude.xml
index 83a39f4..da6867c 100644
--- a/buildtools/src/main/resources/bookkeeper/findbugsExclude.xml
+++ b/buildtools/src/main/resources/bookkeeper/findbugsExclude.xml
@@ -157,4 +157,164 @@
<Match>
<Class name="~org.apache.bookkeeper.proto.statestore.*" />
</Match>
+
+ <!-- Java array itself can be made const but contents for it can always be modified -->
+ <Match>
+ <Class name="org.apache.bookkeeper.statelib.impl.Constants" />
+ <Bug pattern="MS_MUTABLE_ARRAY"/>
+ </Match>
+
+ <Match>
+ <!-- generated code, we can't be held responsible for findbugs in it //-->
+ <Class name="~org\.apache\.distributedlog\.tests\.generated.*" />
+ </Match>
+ <Match>
+ <!-- generated code, we can't be held responsible for findbugs in it //-->
+ <Class name="~org\.apache\.distributedlog\.thrift.*" />
+ </Match>
+ <!-- distributedlog-benchmark -->
+ <Match>
+ <!-- generated code, we can't be held responsible for findbugs in it //-->
+ <Class name="~org\.apache\.distributedlog\.benchmark\.thrift.*" />
+ </Match>
+ <!-- distributedlog-common -->
+ <Match>
+ <Class name="org.apache.distributedlog.util.OrderedScheduler"/>
+ <Bug pattern="NM_SAME_SIMPLE_NAME_AS_SUPERCLASS" />
+ </Match>
+ <!-- distributedlog-core -->
+ <Match>
+ <!-- it is safe to store external bytes reference here. //-->
+ <Class name="org.apache.distributedlog.Entry$Builder" />
+ <Method name="setData" />
+ <Bug pattern="EI_EXPOSE_REP2" />
+ </Match>
+ <Match>
+ <!-- it is safe to store external bytes reference here. //-->
+ <Class name="org.apache.distributedlog.Entry" />
+ <Method name="getRawData" />
+ <Bug pattern="EI_EXPOSE_REP" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.BKAsyncLogReader" />
+ <Method name="run" />
+ <Bug pattern="JLM_JSR166_UTILCONCURRENT_MONITORENTER" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.BKLogReadHandler$1" />
+ <Method name="onSuccess" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.BookKeeperClient$2" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.ReadUtils" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.ReadUtils$2" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.auditor.DLAuditor$2" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.auditor.DLAuditor$8" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.bk.SimpleLedgerAllocator$4" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.bk.SimpleLedgerAllocator$4$1" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.bk.SimpleLedgerAllocator$5" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.impl.acl.ZKAccessControl$4" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.impl.acl.ZKAccessControlManager$1" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.impl.acl.ZKAccessControlManager$1$1" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.impl.metadata.ZKLogStreamMetadataStore$1$1" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.lock.ZKSessionLock" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.lock.ZKSessionLock$12" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.lock.ZKSessionLock$13$1" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.util.Utils" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+ <Match>
+ <Class name="org.apache.distributedlog.util.Utils$6" />
+ <Bug pattern="NP_NONNULL_PARAM_VIOLATION" />
+ </Match>
+ <!-- distributedlog-protocol -->
+ <Match>
+ <!-- it is safe to store external bytes reference here. //-->
+ <Class name="org.apache.distributedlog.LogRecord" />
+ <Bug pattern="EI_EXPOSE_REP2" />
+ </Match>
+ <Match>
+ <!-- it is safe to store external bytes reference here. //-->
+ <Class name="org.apache.distributedlog.LogRecord" />
+ <Method name="getPayload" />
+ <Bug pattern="EI_EXPOSE_REP" />
+ </Match>
+ <!-- distributedlog-proxy-server -->
+ <Match>
+ <!-- generated code, we can't be held responsible for findbugs in it //-->
+ <Class name="~org\.apache\.distributedlog\.service\.placement\.thrift.*" />
+ </Match>
+ <Match>
+ <!-- it is safe to cast exception here. //-->
+ <Class name="org.apache.distributedlog.service.DistributedLogServiceImpl$Stream$2" />
+ <Method name="onFailure" />
+ <Bug pattern="BC_UNCONFIRMED_CAST" />
+ </Match>
+ <Match>
+ <!-- it is safe to cast exception here. //-->
+ <Class name="org.apache.distributedlog.service.stream.BulkWriteOp" />
+ <Method name="isDefiniteFailure" />
+ <Bug pattern="BC_IMPOSSIBLE_INSTANCEOF" />
+ </Match>
+ <!-- distributedlog-messaging -->
+ <Match>
+ <!-- generated code, we can't be held responsible for findbugs in it //-->
+ <Class name="~.*\.TransformedRecord" />
+ </Match>
+ <Match>
+ <!-- it is safe to store external bytes reference here. //-->
+ <Class name="org.apache.distributedlog.messaging.PartitionedMultiWriter" />
+ <Bug pattern="EI_EXPOSE_REP2" />
+ </Match>
+ <Match>
+ <!-- it is safe to store external bytes reference here. //-->
+ <Class name="org.apache.distributedlog.messaging.RRMultiWriter" />
+ <Bug pattern="EI_EXPOSE_REP2" />
+ </Match>
</FindBugsFilter>
diff --git a/gradle.properties b/gradle.properties
index 6caefd9..f3e91a7 100644
--- a/gradle.properties
+++ b/gradle.properties
@@ -24,3 +24,4 @@ apcheRatPluginVersion=0.7.0
shadowPluginVersion=6.1.0
licenseGradlePluginVersion=0.15.0
checkStyleVersion=6.19
+spotbugsPlugin=4.7.0
diff --git a/settings.gradle b/settings.gradle
index fa45234..f6abee5 100644
--- a/settings.gradle
+++ b/settings.gradle
@@ -22,6 +22,7 @@ pluginManagement {
id "org.nosphere.apache.rat" version "${apcheRatPluginVersion}"
id "com.github.johnrengelman.shadow" version "${shadowPluginVersion}"
id "com.github.hierynomus.license" version "${licenseGradlePluginVersion}"
+ id "com.github.spotbugs" version "${spotbugsPlugin}"
}
}
diff --git a/stream/statelib/src/main/java/org/apache/bookkeeper/statelib/impl/mvcc/MVCCStoreImpl.java b/stream/statelib/src/main/java/org/apache/bookkeeper/statelib/impl/mvcc/MVCCStoreImpl.java
index 5c9a83d..74d8628 100644
--- a/stream/statelib/src/main/java/org/apache/bookkeeper/statelib/impl/mvcc/MVCCStoreImpl.java
+++ b/stream/statelib/src/main/java/org/apache/bookkeeper/statelib/impl/mvcc/MVCCStoreImpl.java
@@ -965,7 +965,7 @@ class MVCCStoreImpl<K, V> extends RocksdbKVStore<K, V> implements MVCCStore<K, V
// raw key
byte[] rawKey = (null != key) ? keyCoder.encode(key) : NULL_START_KEY;
-
+ byte[] rawEndKey = NULL_END_KEY;
if (null == endKey) {
// point lookup
MVCCRecord record = getKeyRecord(key, rawKey);
@@ -988,8 +988,9 @@ class MVCCStoreImpl<K, V> extends RocksdbKVStore<K, V> implements MVCCStore<K, V
record.recycle();
}
}
+ } else {
+ rawEndKey = keyCoder.encode(endKey);
}
- byte[] rawEndKey = (null != endKey) ? keyCoder.encode(endKey) : NULL_END_KEY;
Pair<byte[], byte[]> realRange = getRealRange(rawKey, rawEndKey);
rawKey = realRange.getLeft();
rawEndKey = realRange.getRight();
diff --git a/stream/storage/impl/src/main/java/org/apache/bookkeeper/stream/storage/impl/metadata/stream/MetaRangeImpl.java b/stream/storage/impl/src/main/java/org/apache/bookkeeper/stream/storage/impl/metadata/stream/MetaRangeImpl.java
index af7f2d7..6792802 100644
--- a/stream/storage/impl/src/main/java/org/apache/bookkeeper/stream/storage/impl/metadata/stream/MetaRangeImpl.java
+++ b/stream/storage/impl/src/main/java/org/apache/bookkeeper/stream/storage/impl/metadata/stream/MetaRangeImpl.java
@@ -384,7 +384,7 @@ public class MetaRangeImpl implements MetaRange {
}
}
- private void loadStreamMetadata(long streamId, byte[] streamMetadataBytes) {
+ private synchronized void loadStreamMetadata(long streamId, byte[] streamMetadataBytes) {
this.streamId = streamId;
StreamMetadata metadata;
try {