From c7e24c75699a0b55b9f29f3bd2e5f5e2184833f5 Mon Sep 17 00:00:00 2001 From: Tony Kurc Date: Sat, 13 Feb 2016 22:37:10 -0500 Subject: [PATCH] NIFI-1513: fixed some easy to fix errors Addressing checkstyle issue. This closes #221 Signed-off-by: Matt Gilman --- .../apache/nifi/processor/Relationship.java | 2 +- .../functions/GetDelimitedFieldEvaluator.java | 2 +- .../apache/nifi/processor/util/bin/Bin.java | 9 +- .../nifi/remote/client/SiteToSiteClient.java | 2 +- .../manager/impl/WebClusterManager.java | 5 +- .../nifi/processors/script/ExecuteScript.java | 4 +- .../nifi/processors/standard/GetHTTP.java | 110 +++++++++--------- .../nifi/processors/standard/util/Bin.java | 7 +- .../standard/util/crypto/scrypt/Scrypt.java | 2 + .../ControllerStatusReportingTask.java | 1 - .../ganglia/StandardGangliaReporter.java | 10 +- 11 files changed, 82 insertions(+), 72 deletions(-) diff --git a/nifi-api/src/main/java/org/apache/nifi/processor/Relationship.java b/nifi-api/src/main/java/org/apache/nifi/processor/Relationship.java index d9f13be8c8..0fec1f6ef9 100644 --- a/nifi-api/src/main/java/org/apache/nifi/processor/Relationship.java +++ b/nifi-api/src/main/java/org/apache/nifi/processor/Relationship.java @@ -44,7 +44,7 @@ public final class Relationship implements Comparable { protected Relationship(final Builder builder) { this.name = builder.name == null ? null : builder.name.intern(); this.description = builder.description; - this.hashCode = 301 + this.name.hashCode(); // compute only once, since it gets called a bunch and will never change + this.hashCode = 301 + ( (name == null) ? 0 :this.name.hashCode() ); // compute only once, since it gets called a bunch and will never change } @Override diff --git a/nifi-commons/nifi-expression-language/src/main/java/org/apache/nifi/attribute/expression/language/evaluation/functions/GetDelimitedFieldEvaluator.java b/nifi-commons/nifi-expression-language/src/main/java/org/apache/nifi/attribute/expression/language/evaluation/functions/GetDelimitedFieldEvaluator.java index e5695a8330..230b9425f1 100644 --- a/nifi-commons/nifi-expression-language/src/main/java/org/apache/nifi/attribute/expression/language/evaluation/functions/GetDelimitedFieldEvaluator.java +++ b/nifi-commons/nifi-expression-language/src/main/java/org/apache/nifi/attribute/expression/language/evaluation/functions/GetDelimitedFieldEvaluator.java @@ -99,7 +99,7 @@ public class GetDelimitedFieldEvaluator extends StringEvaluator { if (escapeString == null || escapeString.isEmpty()) { throw new AttributeExpressionLanguageException("Cannot evaluate getDelimitedField function because the escape character " + "(which character is used to escape the quote character or delimiter) was not specified"); - } else if (quoteString.length() > 1) { + } else if (escapeString.length() > 1) { throw new AttributeExpressionLanguageException("Cannot evaluate getDelimitedField function because the escape character " + "(which character is used to escape the quote character or delimiter) evaluated to \"" + escapeString + "\", but only a single character is allowed."); } diff --git a/nifi-commons/nifi-processor-utilities/src/main/java/org/apache/nifi/processor/util/bin/Bin.java b/nifi-commons/nifi-processor-utilities/src/main/java/org/apache/nifi/processor/util/bin/Bin.java index af8a8cd895..35c225bfb0 100644 --- a/nifi-commons/nifi-processor-utilities/src/main/java/org/apache/nifi/processor/util/bin/Bin.java +++ b/nifi-commons/nifi-processor-utilities/src/main/java/org/apache/nifi/processor/util/bin/Bin.java @@ -25,6 +25,10 @@ import org.apache.nifi.flowfile.FlowFile; import org.apache.nifi.processor.ProcessSession; import org.apache.nifi.processor.util.FlowFileSessionWrapper; +/** + * Note: {@code Bin} objects are NOT thread safe. If multiple threads access a {@code Bin}, the caller must synchronize + * access. + */ public class Bin { private final long creationMomentEpochNs; @@ -121,8 +125,9 @@ public class Bin { final String countValue = flowFile.getAttribute(fileCountAttribute); final Integer count = toInteger(countValue); if (count != null) { - this.maximumEntries = Math.min(count, this.maximumEntries); - this.minimumEntries = this.maximumEntries; + int currentMaxEntries = this.maximumEntries; + this.maximumEntries = Math.min(count, currentMaxEntries); + this.minimumEntries = currentMaxEntries; } } diff --git a/nifi-commons/nifi-site-to-site-client/src/main/java/org/apache/nifi/remote/client/SiteToSiteClient.java b/nifi-commons/nifi-site-to-site-client/src/main/java/org/apache/nifi/remote/client/SiteToSiteClient.java index 1581c42800..2b04df91d1 100644 --- a/nifi-commons/nifi-site-to-site-client/src/main/java/org/apache/nifi/remote/client/SiteToSiteClient.java +++ b/nifi-commons/nifi-site-to-site-client/src/main/java/org/apache/nifi/remote/client/SiteToSiteClient.java @@ -610,7 +610,7 @@ public interface SiteToSiteClient extends Closeable { trustManagerFactory = null; } - if (keyManagerFactory != null || trustManagerFactory != null) { + if (keyManagerFactory != null && trustManagerFactory != null) { try { // initialize the ssl context final SSLContext sslContext = SSLContext.getInstance("TLS"); diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/manager/impl/WebClusterManager.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/manager/impl/WebClusterManager.java index e98e8e7e72..c0f4c63908 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/manager/impl/WebClusterManager.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/manager/impl/WebClusterManager.java @@ -3015,8 +3015,9 @@ public class WebClusterManager implements HttpClusterManager, ProtocolHandler, C final List summaryDTOs = new ArrayList<>(flowFileSummaries); listingRequest.setFlowFileSummaries(summaryDTOs); - - final int percentCompleted = numStepsCompleted / numStepsTotal; + // depends on invariant if numStepsTotal is 0, so is numStepsCompleted, all steps being completed + // would be 1 + final int percentCompleted = (numStepsTotal == 0) ? 1 : numStepsCompleted / numStepsTotal; listingRequest.setPercentCompleted(percentCompleted); listingRequest.setFinished(finished); diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/ExecuteScript.java b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/ExecuteScript.java index 0af32142e1..3f7c202d51 100644 --- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/ExecuteScript.java +++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/ExecuteScript.java @@ -133,7 +133,9 @@ public class ExecuteScript extends AbstractScriptProcessor { try { if (scriptToRun == null && scriptPath != null) { - scriptToRun = IOUtils.toString(new FileInputStream(scriptPath)); + try (final FileInputStream scriptStream = new FileInputStream(scriptPath)) { + scriptToRun = IOUtils.toString(scriptStream); + } } } catch (IOException ioe) { throw new ProcessException(ioe); diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/GetHTTP.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/GetHTTP.java index 999bead25e..f2ed529047 100644 --- a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/GetHTTP.java +++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/GetHTTP.java @@ -398,9 +398,6 @@ public class GetHTTP extends AbstractSessionFactoryProcessor { clientBuilder.setProxy(new HttpHost(host, port)); } - // create the http client - final CloseableHttpClient client = clientBuilder.build(); - // create request final HttpGet get = new HttpGet(url); get.setConfig(requestConfigBuilder.build()); @@ -426,60 +423,65 @@ public class GetHTTP extends AbstractSessionFactoryProcessor { if (accept != null) { get.addHeader(HEADER_ACCEPT, accept); } - - try { - final StopWatch stopWatch = new StopWatch(true); - final HttpResponse response = client.execute(get); - final int statusCode = response.getStatusLine().getStatusCode(); - if (statusCode == NOT_MODIFIED) { - logger.info("content not retrieved because server returned HTTP Status Code {}: Not Modified", new Object[]{NOT_MODIFIED}); - context.yield(); - // doing a commit in case there were flow files in the input queue - session.commit(); - return; - } - final String statusExplanation = response.getStatusLine().getReasonPhrase(); - - if (statusCode >= 300) { - logger.error("received status code {}:{} from {}", new Object[]{statusCode, statusExplanation, url}); - // doing a commit in case there were flow files in the input queue - session.commit(); - return; - } - - FlowFile flowFile = session.create(); - flowFile = session.putAttribute(flowFile, CoreAttributes.FILENAME.key(), context.getProperty(FILENAME).evaluateAttributeExpressions().getValue()); - flowFile = session.putAttribute(flowFile, this.getClass().getSimpleName().toLowerCase() + ".remote.source", source); - flowFile = session.importFrom(response.getEntity().getContent(), flowFile); - - final Header contentTypeHeader = response.getFirstHeader("Content-Type"); - if (contentTypeHeader != null) { - final String contentType = contentTypeHeader.getValue(); - if (!contentType.trim().isEmpty()) { - flowFile = session.putAttribute(flowFile, CoreAttributes.MIME_TYPE.key(), contentType.trim()); + // create the http client + try ( final CloseableHttpClient client = clientBuilder.build() ) { + // NOTE: including this inner try in order to swallow exceptions on close + try { + final StopWatch stopWatch = new StopWatch(true); + final HttpResponse response = client.execute(get); + final int statusCode = response.getStatusLine().getStatusCode(); + if (statusCode == NOT_MODIFIED) { + logger.info("content not retrieved because server returned HTTP Status Code {}: Not Modified", new Object[]{NOT_MODIFIED}); + context.yield(); + // doing a commit in case there were flow files in the input queue + session.commit(); + return; } + final String statusExplanation = response.getStatusLine().getReasonPhrase(); + + if (statusCode >= 300) { + logger.error("received status code {}:{} from {}", new Object[]{statusCode, statusExplanation, url}); + // doing a commit in case there were flow files in the input queue + session.commit(); + return; + } + + FlowFile flowFile = session.create(); + flowFile = session.putAttribute(flowFile, CoreAttributes.FILENAME.key(), context.getProperty(FILENAME).evaluateAttributeExpressions().getValue()); + flowFile = session.putAttribute(flowFile, this.getClass().getSimpleName().toLowerCase() + ".remote.source", source); + flowFile = session.importFrom(response.getEntity().getContent(), flowFile); + + final Header contentTypeHeader = response.getFirstHeader("Content-Type"); + if (contentTypeHeader != null) { + final String contentType = contentTypeHeader.getValue(); + if (!contentType.trim().isEmpty()) { + flowFile = session.putAttribute(flowFile, CoreAttributes.MIME_TYPE.key(), contentType.trim()); + } + } + + final long flowFileSize = flowFile.getSize(); + stopWatch.stop(); + final String dataRate = stopWatch.calculateDataRate(flowFileSize); + session.getProvenanceReporter().receive(flowFile, url, stopWatch.getDuration(TimeUnit.MILLISECONDS)); + session.transfer(flowFile, REL_SUCCESS); + logger.info("Successfully received {} from {} at a rate of {}; transferred to success", new Object[]{flowFile, url, dataRate}); + session.commit(); + + updateStateMap(context,response,beforeStateMap,url); + + } catch (final IOException e) { + context.yield(); + session.rollback(); + logger.error("Failed to retrieve file from {} due to {}; rolling back session", new Object[]{url, e.getMessage()}, e); + throw new ProcessException(e); + } catch (final Throwable t) { + context.yield(); + session.rollback(); + logger.error("Failed to process due to {}; rolling back session", new Object[]{t.getMessage()}, t); + throw t; } - - final long flowFileSize = flowFile.getSize(); - stopWatch.stop(); - final String dataRate = stopWatch.calculateDataRate(flowFileSize); - session.getProvenanceReporter().receive(flowFile, url, stopWatch.getDuration(TimeUnit.MILLISECONDS)); - session.transfer(flowFile, REL_SUCCESS); - logger.info("Successfully received {} from {} at a rate of {}; transferred to success", new Object[]{flowFile, url, dataRate}); - session.commit(); - - updateStateMap(context,response,beforeStateMap,url); - } catch (final IOException e) { - context.yield(); - session.rollback(); - logger.error("Failed to retrieve file from {} due to {}; rolling back session", new Object[]{url, e.getMessage()}, e); - throw new ProcessException(e); - } catch (final Throwable t) { - context.yield(); - session.rollback(); - logger.error("Failed to process due to {}; rolling back session", new Object[]{t.getMessage()}, t); - throw t; + logger.debug("Error closing client due to {}, continuing.", new Object[]{e.getMessage()}); } } finally { conMan.shutdown(); diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/util/Bin.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/util/Bin.java index 96c2a40ebb..09cccbdd83 100644 --- a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/util/Bin.java +++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/util/Bin.java @@ -25,6 +25,8 @@ import org.apache.nifi.flowfile.FlowFile; import org.apache.nifi.processor.ProcessSession; /** + * Note: {@code Bin} objects are NOT thread safe. If multiple threads access a {@code Bin}, the caller must synchronize + * access. * @deprecated As of release 0.5.0, replaced by * {@link org.apache.nifi.processor.util.bin.Bin} */ @@ -125,8 +127,9 @@ public class Bin { final String countValue = flowFile.getAttribute(fileCountAttribute); final Integer count = toInteger(countValue); if (count != null) { - this.maximumEntries = Math.min(count, this.maximumEntries); - this.minimumEntries = this.maximumEntries; + final int currentMaximumEntries = this.maximumEntries; + this.maximumEntries = Math.min(count, currentMaximumEntries); + this.minimumEntries = currentMaximumEntries; } } diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/util/crypto/scrypt/Scrypt.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/util/crypto/scrypt/Scrypt.java index b1cdbee66b..7785e9eca7 100644 --- a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/util/crypto/scrypt/Scrypt.java +++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/util/crypto/scrypt/Scrypt.java @@ -285,6 +285,8 @@ public class Scrypt { // Do not enforce this check here. According to the scrypt spec, the salt can be empty. However, in the user-facing ScryptCipherProvider, enforce an arbitrary check to avoid empty salts logger.warn("An empty salt was used for scrypt key derivation"); // throw new IllegalArgumentException("Salt cannot be empty"); + // as the Exception is not being thrown, prevent NPE if salt is null by setting it to empty array + if( salt == null ) salt = new byte[]{}; } if (saltLength < 8 || saltLength > 32) { diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/src/main/java/org/apache/nifi/controller/ControllerStatusReportingTask.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/src/main/java/org/apache/nifi/controller/ControllerStatusReportingTask.java index 4c9b4b1306..49cb9de90f 100644 --- a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/src/main/java/org/apache/nifi/controller/ControllerStatusReportingTask.java +++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/src/main/java/org/apache/nifi/controller/ControllerStatusReportingTask.java @@ -105,7 +105,6 @@ public class ControllerStatusReportingTask extends AbstractReportingTask { @Override public void onTrigger(final ReportingContext context) { final ProcessGroupStatus controllerStatus = context.getEventAccess().getControllerStatus(); - controllerStatus.clone(); final boolean showDeltas = context.getProperty(SHOW_DELTAS).asBoolean(); diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/src/main/java/org/apache/nifi/reporting/ganglia/StandardGangliaReporter.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/src/main/java/org/apache/nifi/reporting/ganglia/StandardGangliaReporter.java index 54e4bd30cd..035cd3cac7 100644 --- a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/src/main/java/org/apache/nifi/reporting/ganglia/StandardGangliaReporter.java +++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/src/main/java/org/apache/nifi/reporting/ganglia/StandardGangliaReporter.java @@ -117,8 +117,7 @@ public class StandardGangliaReporter extends AbstractReportingTask { return 0L; } - final Long value = status.getBytesReceived(); - return (value == null) ? 0L : value; + return status.getBytesReceived(); } }); @@ -130,8 +129,7 @@ public class StandardGangliaReporter extends AbstractReportingTask { return 0; } - final Integer value = status.getFlowFilesSent(); - return (value == null) ? 0 : value; + return status.getFlowFilesSent(); } }); @@ -142,9 +140,7 @@ public class StandardGangliaReporter extends AbstractReportingTask { if (status == null) { return 0L; } - - final Long value = status.getBytesSent(); - return (value == null) ? 0L : value; + return status.getBytesSent(); } });