[
https://issues.apache.org/jira/browse/BEAM-4324?focusedWorklogId=106710&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-106710
]
ASF GitHub Bot logged work on BEAM-4324:
----------------------------------------
Author: ASF GitHub Bot
Created on: 29/May/18 16:52
Start Date: 29/May/18 16:52
Worklog Time Spent: 10m
Work Description: kennknowles closed pull request #5502: [BEAM-4324]
Enforce ErrorProne analysis in sorter extensions
URL: https://github.com/apache/beam/pull/5502
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/sdks/java/extensions/sorter/build.gradle
b/sdks/java/extensions/sorter/build.gradle
index f38acfed291..0ff8c5e60ff 100644
--- a/sdks/java/extensions/sorter/build.gradle
+++ b/sdks/java/extensions/sorter/build.gradle
@@ -17,12 +17,13 @@
*/
apply from: project(":").file("build_rules.gradle")
-applyJavaNature()
+applyJavaNature(failOnWarning: true)
description = "Apache Beam :: SDKs :: Java :: Extensions :: Sorter"
dependencies {
compile library.java.guava
+ compileOnly library.java.findbugs_annotations
shadow project(path: ":beam-sdks-java-core", configuration: "shadow")
provided library.java.hadoop_mapreduce_client_core
provided library.java.hadoop_common
@@ -30,4 +31,5 @@ dependencies {
testCompile library.java.hamcrest_core
testCompile library.java.mockito_core
testCompile library.java.junit
+ testCompileOnly library.java.findbugs_annotations
}
diff --git
a/sdks/java/extensions/sorter/src/main/java/org/apache/beam/sdk/extensions/sorter/BufferedExternalSorter.java
b/sdks/java/extensions/sorter/src/main/java/org/apache/beam/sdk/extensions/sorter/BufferedExternalSorter.java
index efa251a797a..601391a5d88 100644
---
a/sdks/java/extensions/sorter/src/main/java/org/apache/beam/sdk/extensions/sorter/BufferedExternalSorter.java
+++
b/sdks/java/extensions/sorter/src/main/java/org/apache/beam/sdk/extensions/sorter/BufferedExternalSorter.java
@@ -76,7 +76,7 @@ public int getMemoryMB() {
}
}
- private ExternalSorter externalSorter;
+ private final ExternalSorter externalSorter;
private InMemorySorter inMemorySorter;
boolean inMemorySorterFull;
diff --git
a/sdks/java/extensions/sorter/src/main/java/org/apache/beam/sdk/extensions/sorter/ExternalSorter.java
b/sdks/java/extensions/sorter/src/main/java/org/apache/beam/sdk/extensions/sorter/ExternalSorter.java
index 492efba774c..9f2752aba21 100644
---
a/sdks/java/extensions/sorter/src/main/java/org/apache/beam/sdk/extensions/sorter/ExternalSorter.java
+++
b/sdks/java/extensions/sorter/src/main/java/org/apache/beam/sdk/extensions/sorter/ExternalSorter.java
@@ -30,6 +30,7 @@
import java.util.Iterator;
import java.util.NoSuchElementException;
import java.util.UUID;
+import javax.annotation.Nonnull;
import org.apache.beam.sdk.values.KV;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
@@ -42,7 +43,7 @@
/** Does an external sort of the provided values using Hadoop's {@link
SequenceFile}. */
class ExternalSorter implements Sorter {
- private Options options;
+ private final Options options;
/** Whether {@link #sort()} was already called. */
private boolean sortCalled = false;
@@ -170,6 +171,7 @@ private void initHadoopSorter() throws IOException {
/** An {@link Iterable} producing the iterators over sorted data. */
private class SortedRecordsIterable implements Iterable<KV<byte[], byte[]>> {
+ @Nonnull
@Override
public Iterator<KV<byte[], byte[]>> iterator() {
return new SortedRecordsIterator();
@@ -181,7 +183,7 @@ private void initHadoopSorter() throws IOException {
private RawKeyValueIterator iterator;
/** Next {@link KV} to return from {@link #next()}. */
- private KV<byte[], byte[]> nextKV = null;
+ private KV<byte[], byte[]> nextKV;
SortedRecordsIterator() {
try {
diff --git
a/sdks/java/extensions/sorter/src/main/java/org/apache/beam/sdk/extensions/sorter/InMemorySorter.java
b/sdks/java/extensions/sorter/src/main/java/org/apache/beam/sdk/extensions/sorter/InMemorySorter.java
index b23bb2d16ef..ab4b753ff90 100644
---
a/sdks/java/extensions/sorter/src/main/java/org/apache/beam/sdk/extensions/sorter/InMemorySorter.java
+++
b/sdks/java/extensions/sorter/src/main/java/org/apache/beam/sdk/extensions/sorter/InMemorySorter.java
@@ -69,7 +69,7 @@ public long getMemoryMB() {
private static final long RECORD_MEMORY_OVERHEAD_ESTIMATE = 11 *
NUM_BYTES_PER_WORD;
/** Maximum size of the buffer in bytes. */
- private long maxBufferSize;
+ private final long maxBufferSize;
/** Current number of stored bytes. Including estimated overhead bytes. */
private long numBytes;
@@ -78,7 +78,7 @@ public long getMemoryMB() {
private boolean sortCalled;
/** The stored records to be sorted. */
- private ArrayList<KV<byte[], byte[]>> records = new ArrayList<>();
+ private final ArrayList<KV<byte[], byte[]>> records = new ArrayList<>();
/** Private constructor. */
private InMemorySorter(Options options) {
diff --git
a/sdks/java/extensions/sorter/src/main/java/org/apache/beam/sdk/extensions/sorter/SortValues.java
b/sdks/java/extensions/sorter/src/main/java/org/apache/beam/sdk/extensions/sorter/SortValues.java
index d93b7807427..5e5fad87cbc 100644
---
a/sdks/java/extensions/sorter/src/main/java/org/apache/beam/sdk/extensions/sorter/SortValues.java
+++
b/sdks/java/extensions/sorter/src/main/java/org/apache/beam/sdk/extensions/sorter/SortValues.java
@@ -20,6 +20,7 @@
import java.io.IOException;
import java.util.Iterator;
+import javax.annotation.Nonnull;
import org.apache.beam.sdk.coders.Coder;
import org.apache.beam.sdk.coders.IterableCoder;
import org.apache.beam.sdk.coders.KvCoder;
@@ -53,7 +54,7 @@
PCollection<KV<PrimaryKeyT, Iterable<KV<SecondaryKeyT, ValueT>>>>,
PCollection<KV<PrimaryKeyT, Iterable<KV<SecondaryKeyT, ValueT>>>>> {
- private BufferedExternalSorter.Options sorterOptions;
+ private final BufferedExternalSorter.Options sorterOptions;
private SortValues(BufferedExternalSorter.Options sorterOptions) {
this.sorterOptions = sorterOptions;
@@ -154,22 +155,20 @@ public void processElement(ProcessContext c) {
CoderUtils.encodeToByteArray(valueCoder,
record.getValue())));
}
- c.output(
- KV.of(
- c.element().getKey(),
- (Iterable<KV<SecondaryKeyT, ValueT>>) (new
DecodingIterable(sorter.sort()))));
+ c.output(KV.of(c.element().getKey(), new
DecodingIterable(sorter.sort())));
} catch (IOException e) {
throw new RuntimeException(e);
}
}
private class DecodingIterable implements Iterable<KV<SecondaryKeyT,
ValueT>> {
- Iterable<KV<byte[], byte[]>> iterable;
+ final Iterable<KV<byte[], byte[]>> iterable;
DecodingIterable(Iterable<KV<byte[], byte[]>> iterable) {
this.iterable = iterable;
}
+ @Nonnull
@Override
public Iterator<KV<SecondaryKeyT, ValueT>> iterator() {
return new DecodingIterator(iterable.iterator());
@@ -177,7 +176,7 @@ public void processElement(ProcessContext c) {
}
private class DecodingIterator implements Iterator<KV<SecondaryKeyT,
ValueT>> {
- Iterator<KV<byte[], byte[]>> iterator;
+ final Iterator<KV<byte[], byte[]>> iterator;
DecodingIterator(Iterator<KV<byte[], byte[]>> iterator) {
this.iterator = iterator;
diff --git
a/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/BufferedExternalSorterTest.java
b/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/BufferedExternalSorterTest.java
index 234c705cc26..b8ad726f2e7 100644
---
a/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/BufferedExternalSorterTest.java
+++
b/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/BufferedExternalSorterTest.java
@@ -47,7 +47,7 @@
@RunWith(JUnit4.class)
public class BufferedExternalSorterTest {
@Rule public ExpectedException thrown = ExpectedException.none();
- static Path tmpLocation;
+ private static Path tmpLocation;
@BeforeClass
public static void setupTempDir() throws IOException {
@@ -198,7 +198,7 @@ public void testSortTwice() throws Exception {
}
@Test
- public void testNegativeMemory() throws Exception {
+ public void testNegativeMemory() {
thrown.expect(IllegalArgumentException.class);
thrown.expectMessage("memoryMB must be greater than zero");
BufferedExternalSorter.Options options = BufferedExternalSorter.options()
@@ -207,7 +207,7 @@ public void testNegativeMemory() throws Exception {
}
@Test
- public void testZeroMemory() throws Exception {
+ public void testZeroMemory() {
thrown.expect(IllegalArgumentException.class);
thrown.expectMessage("memoryMB must be greater than zero");
BufferedExternalSorter.Options options = BufferedExternalSorter.options();
@@ -215,7 +215,7 @@ public void testZeroMemory() throws Exception {
}
@Test
- public void testMemoryTooLarge() throws Exception {
+ public void testMemoryTooLarge() {
thrown.expect(IllegalArgumentException.class);
thrown.expectMessage("memoryMB must be less than 2048");
BufferedExternalSorter.Options options = BufferedExternalSorter.options();
diff --git
a/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/ExternalSorterTest.java
b/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/ExternalSorterTest.java
index d39764e85a0..ba4f68709fb 100644
---
a/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/ExternalSorterTest.java
+++
b/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/ExternalSorterTest.java
@@ -38,7 +38,7 @@
@RunWith(JUnit4.class)
public class ExternalSorterTest {
@Rule public ExpectedException thrown = ExpectedException.none();
- static Path tmpLocation;
+ private static Path tmpLocation;
@BeforeClass
public static void setupTempDir() throws IOException {
@@ -111,7 +111,7 @@ public void testSortTwice() throws Exception {
}
@Test
- public void testNegativeMemory() throws Exception {
+ public void testNegativeMemory() {
thrown.expect(IllegalArgumentException.class);
thrown.expectMessage("memoryMB must be greater than zero");
ExternalSorter.Options options = new ExternalSorter.Options();
@@ -119,7 +119,7 @@ public void testNegativeMemory() throws Exception {
}
@Test
- public void testZeroMemory() throws Exception {
+ public void testZeroMemory() {
thrown.expect(IllegalArgumentException.class);
thrown.expectMessage("memoryMB must be greater than zero");
ExternalSorter.Options options = new ExternalSorter.Options();
@@ -127,7 +127,7 @@ public void testZeroMemory() throws Exception {
}
@Test
- public void testMemoryTooLarge() throws Exception {
+ public void testMemoryTooLarge() {
thrown.expect(IllegalArgumentException.class);
thrown.expectMessage("memoryMB must be less than 2048");
ExternalSorter.Options options = new ExternalSorter.Options();
diff --git
a/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/InMemorySorterTest.java
b/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/InMemorySorterTest.java
index d35df28847b..55657af5e32 100644
---
a/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/InMemorySorterTest.java
+++
b/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/InMemorySorterTest.java
@@ -74,8 +74,6 @@ public void testSortTwice() throws Exception {
/**
* Verify an exception is thrown when the in memory sorter runs out of space.
- *
- * @throws Exception
*/
@Test
public void testOutOfSpace() throws Exception {
@@ -92,7 +90,7 @@ public void testOutOfSpace() throws Exception {
}
@Test
- public void testAddIfRoom() throws Exception {
+ public void testAddIfRoom() {
InMemorySorter.Options options = new InMemorySorter.Options();
options.setMemoryMB(1);
InMemorySorter sorter = InMemorySorter.create(options);
@@ -106,7 +104,7 @@ public void testAddIfRoom() throws Exception {
}
@Test
- public void testAddIfRoomOverhead() throws Exception {
+ public void testAddIfRoomOverhead() {
InMemorySorter.Options options = new InMemorySorter.Options();
options.setMemoryMB(1);
InMemorySorter sorter = InMemorySorter.create(options);
@@ -124,7 +122,7 @@ public void testAddIfRoomOverhead() throws Exception {
}
@Test
- public void testNegativeMemory() throws Exception {
+ public void testNegativeMemory() {
thrown.expect(IllegalArgumentException.class);
thrown.expectMessage("memoryMB must be greater than zero");
InMemorySorter.Options options = new InMemorySorter.Options();
@@ -132,7 +130,7 @@ public void testNegativeMemory() throws Exception {
}
@Test
- public void testZeroMemory() throws Exception {
+ public void testZeroMemory() {
thrown.expect(IllegalArgumentException.class);
thrown.expectMessage("memoryMB must be greater than zero");
InMemorySorter.Options options = new InMemorySorter.Options();
diff --git
a/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/SortValuesTest.java
b/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/SortValuesTest.java
index 247f1a4a16c..9016f8d95ab 100644
---
a/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/SortValuesTest.java
+++
b/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/SortValuesTest.java
@@ -47,7 +47,7 @@
public final transient TestPipeline p = TestPipeline.create();
@Test
- public void testSecondaryKeySorting() throws Exception {
+ public void testSecondaryKeySorting() {
// Create a PCollection of <Key, <SecondaryKey, Value>> pairs.
PCollection<KV<String, KV<String, Integer>>> input =
p.apply(
@@ -97,15 +97,15 @@ public Void apply(Iterable<KV<String, Iterable<KV<String,
Integer>>>> actual) {
}
/** Matcher for KVs. Forked from Beam's org/apache/beam/sdk/TestUtils.java */
- public static class KvMatcher<K, V> extends TypeSafeMatcher<KV<? extends K,
? extends V>> {
+ static class KvMatcher<K, V> extends TypeSafeMatcher<KV<? extends K, ?
extends V>> {
final Matcher<? super K> keyMatcher;
final Matcher<? super V> valueMatcher;
- public static <K, V> KvMatcher<K, V> isKv(Matcher<K> keyMatcher,
Matcher<V> valueMatcher) {
+ static <K, V> KvMatcher<K, V> isKv(Matcher<K> keyMatcher, Matcher<V>
valueMatcher) {
return new KvMatcher<>(keyMatcher, valueMatcher);
}
- public KvMatcher(Matcher<? super K> keyMatcher, Matcher<? super V>
valueMatcher) {
+ KvMatcher(Matcher<? super K> keyMatcher, Matcher<? super V> valueMatcher) {
this.keyMatcher = keyMatcher;
this.valueMatcher = valueMatcher;
}
diff --git
a/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/SorterTestUtils.java
b/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/SorterTestUtils.java
index 3ab8b348cf3..ea4bb5b2f13 100644
---
a/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/SorterTestUtils.java
+++
b/sdks/java/extensions/sorter/src/test/java/org/apache/beam/sdk/extensions/sorter/SorterTestUtils.java
@@ -30,7 +30,7 @@
import org.junit.rules.ExpectedException;
/** A set of basic tests for {@link Sorter}s. */
-public class SorterTestUtils {
+class SorterTestUtils {
public static void testEmpty(Sorter sorter) throws Exception {
assertThat(sorter.sort(), is(emptyIterable()));
@@ -69,7 +69,7 @@ public static void testMultipleIterations(Sorter sorter)
throws Exception {
/** Class that generates a new sorter. Used when performance testing
multiple sorter creation. */
interface SorterGenerator {
- Sorter generateSorter() throws Exception;
+ Sorter generateSorter();
}
/**
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 106710)
Time Spent: 0.5h (was: 20m)
> Enforce ErrorProne analysis in sorter extensions project
> --------------------------------------------------------
>
> Key: BEAM-4324
> URL: https://issues.apache.org/jira/browse/BEAM-4324
> Project: Beam
> Issue Type: Improvement
> Components: sdk-java-core
> Reporter: Scott Wegner
> Assignee: Ismaël Mejía
> Priority: Minor
> Labels: errorprone, starter
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Java ErrorProne static analysis was [recently
> enabled|https://github.com/apache/beam/pull/5161] in the Gradle build
> process, but only as warnings. ErrorProne errors are generally useful and
> easy to fix. Some work was done to [make sdks-java-core
> ErrorProne-clean|https://github.com/apache/beam/pull/5319] and add
> enforcement. This task is clean ErrorProne warnings and add enforcement in
> {{beam-sdks-java-extensions-sorter}}. Additional context discussed on the
> [dev
> list|https://lists.apache.org/thread.html/95aae2785c3cd728c2d3378cbdff2a7ba19caffcd4faa2049d2e2f46@%3Cdev.beam.apache.org%3E].
> Fixing this issue will involve:
> # Follow instructions in the [Contribution
> Guide|https://beam.apache.org/contribute/] to set up a {{beam}} development
> environment.
> # Run the following command to compile and run ErrorProne analysis on the
> project: {{./gradlew :beam-sdks-java-extensions-sorter:assemble}}
> # Fix each ErrorProne warning from the {{sdks/java/extensions/sorter}}
> project.
> # In {{sdks/java/extensions/sorter/build.gradle}}, add {{failOnWarning:
> true}} to the call the {{applyJavaNature()}}
> ([example|https://github.com/apache/beam/pull/5319/files#diff-9390c20635aed5f42f83b97506a87333R20]).
> This starter issue is sponsored by [~swegner]. Feel free to [reach
> out|https://beam.apache.org/community/contact-us/] with questions or code
> review:
> * JIRA: [~swegner]
> * GitHub: [@swegner|https://github.com/swegner]
> * Slack: [@Scott Wegner|https://s.apache.org/beam-slack-channel]
> * Email: swegner at google dot com
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
