Skip to content

Commit

Permalink
Resolve most HuntBugs warnings (including two bugs fixed) (#56)
Browse files Browse the repository at this point in the history
* Address some HuntBugs warnings

* Add test for Comparators.inverse() overflow

* Add deprecation, why LimitedYieldingAccumulator is deprecated

* Remove unused import
  • Loading branch information
leventov authored and drcrallen committed Oct 13, 2016
1 parent 3c7b49c commit 826021f
Show file tree
Hide file tree
Showing 12 changed files with 26 additions and 17 deletions.
2 changes: 1 addition & 1 deletion src/main/java/com/metamx/common/CompressionUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public FileUtils.FileCopyResult call() throws Exception
} else {
final File tmpFile = File.createTempFile("compressionUtilZipCache", ZIP_SUFFIX);
try {
FileUtils.FileCopyResult copyResult = FileUtils.retryCopy(
FileUtils.retryCopy(
byteSource,
tmpFile,
shouldRetry,
Expand Down
3 changes: 0 additions & 3 deletions src/main/java/com/metamx/common/Granularity.java
Original file line number Diff line number Diff line change
Expand Up @@ -450,9 +450,6 @@ public DateTime toDate(String filePath, Formatter formatter)
},
WEEK
{
DateTimeFormatter defaultFormat = DateTimeFormat.forPattern("'y'=yyyy/'m'=MM/'d'=dd");
DateTimeFormatter hiveFormat = DateTimeFormat.forPattern("'dt'=yyyy-MM-dd");
DateTimeFormatter lowerDefaultFormat = DateTimeFormat.forPattern("'y'=yyyy/'m'=MM/'d'=dd");

@Override
public DateTimeFormatter getFormatter(Formatter type)
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/metamx/common/RetryUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
import com.google.common.base.Throwables;
import com.metamx.common.logger.Logger;

import java.util.Random;
import java.util.concurrent.Callable;
import java.util.concurrent.ThreadLocalRandom;

public class RetryUtils
{
Expand Down Expand Up @@ -82,7 +82,7 @@ private static void awaitNextRetry(final Throwable e, final int nTry, final bool
{
final long baseSleepMillis = 1000;
final long maxSleepMillis = 60000;
final double fuzzyMultiplier = Math.min(Math.max(1 + 0.2 * new Random().nextGaussian(), 0), 2);
final double fuzzyMultiplier = Math.min(Math.max(1 + 0.2 * ThreadLocalRandom.current().nextGaussian(), 0), 2);
final long sleepMillis = (long) (Math.min(maxSleepMillis, baseSleepMillis * Math.pow(2, nTry - 1))
* fuzzyMultiplier);
if (quiet) {
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/metamx/common/StreamUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,9 @@ public static long copyAndClose(InputStream is, OutputStream os) throws IOExcept
public static long copyWithTimeout(InputStream is, OutputStream os, long timeout) throws IOException, TimeoutException
{
byte[] buffer = new byte[DEFAULT_BUFFER_SIZE];
int n = 0;
int n;
long startTime = System.currentTimeMillis();
long size = 0l;
long size = 0;
while (-1 != (n = is.read(buffer))) {
if (System.currentTimeMillis() - startTime > timeout) {
throw new TimeoutException(String.format("Copy time has exceeded %,d millis", timeout));
Expand Down
1 change: 0 additions & 1 deletion src/main/java/com/metamx/common/StringUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
*/
public class StringUtils
{
private static final Logger log = new Logger(StringUtils.class);
@Deprecated // Charset parameters to String are currently slower than the charset's string name
public static final Charset UTF8_CHARSET = Charsets.UTF_8;
public static final String UTF8_STRING = com.google.common.base.Charsets.UTF_8.toString();
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/metamx/common/guava/Comparators.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public static <T> Comparator<T> inverse(final Comparator<T> baseComp)
@Override
public int compare(T t, T t1)
{
return - baseComp.compare(t, t1);
return baseComp.compare(t1, t);
}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
package com.metamx.common.guava;

/**
* @deprecated this class uses expensive volatile counter inside, but it is not thread-safe. It is going to be removed
* in the future.
*/
@Deprecated
public class LimitedYieldingAccumulator<OutType, T> extends YieldingAccumulator<OutType, T>
{
private final int limit;
Expand Down
1 change: 0 additions & 1 deletion src/main/java/com/metamx/common/parsers/CSVParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@

public class CSVParser implements Parser<String, Object>
{
private static final Logger log = new Logger(CSVParser.class);

private final String listDelimiter;
private final Splitter listSplitter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@

public class DelimitedParser implements Parser<String, Object>
{
private static final Logger log = new Logger(DelimitedParser.class);
private static final String DEFAULT_DELIMITER = "\t";

private final String delimiter;
Expand Down
7 changes: 3 additions & 4 deletions src/main/java/com/metamx/common/parsers/JSONPathParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
public class JSONPathParser implements Parser<String, Object>
{
private final Map<String, Pair<FieldType, JsonPath>> fieldPathMap;
private final List<FieldSpec> fieldSpecs;
private final boolean useFieldDiscovery;
private final ObjectMapper mapper;
private final CharsetEncoder enc = Charsets.UTF_8.newEncoder();
Expand All @@ -60,7 +59,6 @@ public class JSONPathParser implements Parser<String, Object>
*/
public JSONPathParser(List<FieldSpec> fieldSpecs, boolean useFieldDiscovery, ObjectMapper mapper)
{
this.fieldSpecs = fieldSpecs;
this.fieldPathMap = generateFieldPaths(fieldSpecs);
this.useFieldDiscovery = useFieldDiscovery;
this.mapper = mapper == null ? new ObjectMapper() : mapper;
Expand Down Expand Up @@ -145,9 +143,10 @@ private Map<String, Pair<FieldType, JsonPath>> generateFieldPaths(List<FieldSpec

private void discoverFields(Map<String, Object> map, Map<String, Object> document)
{
for (String field : document.keySet()) {
for (Map.Entry<String, Object> e : document.entrySet()) {
String field = e.getKey();
if (!map.containsKey(field)) {
Object val = document.get(field);
Object val = e.getValue();
if (val == null) {
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@

public class TimestampParser
{
private static final Logger log = new Logger(TimestampParser.class);

public static Function<String, DateTime> createTimestampParser(
final String format
Expand Down
14 changes: 14 additions & 0 deletions src/test/java/com/metamx/common/guava/ComparatorsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,20 @@ public void testInverse() throws Exception
Assert.assertEquals(0, inverted.compare(1, 1));
}

@Test
public void testInverseOverflow()
{
Comparator<Integer> invertedSimpleIntegerComparator = Comparators.inverse(new Comparator<Integer>()
{
@Override
public int compare(Integer o1, Integer o2)
{
return o1 - o2;
}
});
Assert.assertTrue(invertedSimpleIntegerComparator.compare(0, Integer.MIN_VALUE) < 0);
}

@Test
public void testIntervalsByStartThenEnd() throws Exception
{
Expand Down

0 comments on commit 826021f

Please sign in to comment.