Skip to content

Commit

Permalink
fixed sonar bugs
Browse files Browse the repository at this point in the history
  • Loading branch information
@timbo2k
timbo2k committed Mar 11, 2019
1 parent 1438150 commit de8877f
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 18 deletions.
2 changes: 1 addition & 1 deletion plc4j/utils/scraper/src/main/java/org/apache/plc4x/java/scraper/ScraperTaskImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public void run() {
try {
connection.close();
} catch (Exception e) {
// intentionally do nothing
LOGGER.warn("Error on closing connection",e);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...er/src/main/java/org/apache/plc4x/java/scraper/triggeredscraper/TriggeredScraperImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public void start() {
// Store the handle for stopping, etc.
futures.put(task, future);
} catch (ScraperException e) {
LOGGER.warn("Error executing the job {} for conn {} ({}) at rate {} ms",tuple.getLeft().getJobName(), tuple.getMiddle(), tuple.getRight(), tuple.getLeft().getScrapeRate());
LOGGER.warn("Error executing the job {} for conn {} ({}) at rate {} ms",tuple.getLeft().getJobName(), tuple.getMiddle(), tuple.getRight(), tuple.getLeft().getScrapeRate(),e);
}

}
Expand Down
14 changes: 7 additions & 7 deletions ...er/src/main/java/org/apache/plc4x/java/scraper/triggeredscraper/TriggeredScraperTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
* ToDo Implement the monitoring as well: PLC4X-90
*/
public class TriggeredScraperTask implements ScraperTask {
private static final Logger logger = LoggerFactory.getLogger(TriggeredScraperTask.class);
private static final Logger LOGGER = LoggerFactory.getLogger(TriggeredScraperTask.class);

private final PlcDriverManager driverManager;
private final String jobName;
Expand Down Expand Up @@ -91,7 +91,7 @@ public TriggeredScraperTask(PlcDriverManager driverManager,
public void run() {
if(this.triggerHandler.checkTrigger()) {
// Does a single fetch only when trigger is valid
logger.trace("Start new scrape of task of job {} for connection {}", jobName, connectionAlias);
LOGGER.trace("Start new scrape of task of job {} for connection {}", jobName, connectionAlias);
requestCounter.incrementAndGet();
StopWatch stopWatch = new StopWatch();
stopWatch.start();
Expand All @@ -101,17 +101,17 @@ public void run() {
try {
return driverManager.getConnection(connectionString);
} catch (PlcConnectionException e) {
logger.warn("Unable to instantiate connection to " + connectionString, e);
LOGGER.warn("Unable to instantiate connection to " + connectionString, e);
throw new PlcRuntimeException(e);
}
}, executorService);
connection = future.get(10 * requestTimeoutMs, TimeUnit.MILLISECONDS);
logger.trace("Connection to {} established: {}", connectionString, connection);
LOGGER.trace("Connection to {} established: {}", connectionString, connection);
PlcReadResponse response;
try {
PlcReadRequest.Builder builder = connection.readRequestBuilder();
fields.forEach((alias, qry) -> {
logger.trace("Requesting: {} -> {}", alias, qry);
LOGGER.trace("Requesting: {} -> {}", alias, qry);
builder.addItem(alias, qry);
});
response = builder
Expand All @@ -133,14 +133,14 @@ public void run() {
// Handle response (Async)
CompletableFuture.runAsync(() -> resultHandler.handle(jobName, connectionAlias, transformResponseToMap(response)), executorService);
} catch (Exception e) {
logger.debug("Exception during scrape", e);
LOGGER.debug("Exception during scrape", e);
handleException(e);
} finally {
if (connection != null) {
try {
connection.close();
} catch (Exception e) {
// intentionally do nothing
LOGGER.warn("Error on closing connection",e);
}
}
}
Expand Down
28 changes: 22 additions & 6 deletions ...a/org/apache/plc4x/java/scraper/triggeredscraper/triggerhandler/TriggerConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,17 @@
*/
//ToDo: Improve structure to make it more generic --> PLC4X-89
public class TriggerConfiguration{
private static final Logger logger = LoggerFactory.getLogger(TriggerConfiguration.class);
private static final Logger LOGGER = LoggerFactory.getLogger(TriggerConfiguration.class);

private static final String S_7_TRIGGER_VAR = "S7_TRIGGER_VAR";
private static final String SCHEDULED = "SCHEDULED";

private static final double TOLERANCE_FLOATING_EQUALITY = 1e-6;

private static final Pattern TRIGGER_STRATEGY_PATTERN =
Pattern.compile("\\((?<strategy>[A-Z_0-9]+),(?<scheduledInterval>\\d+)(,(\\((?<triggerVar>\\S+)\\))((?<comp>[!=<>]{1,2}))(\\((?<compVar>[a-z0-9.\\-]+)\\)))?\\)");


private final TriggerType triggerType;
private final Long scrapeInterval;
private final String triggerVariable;
Expand Down Expand Up @@ -77,7 +80,7 @@ public TriggerConfiguration(TriggerType triggerType, String scrapeInterval, Stri
try {
this.plcField = S7Field.of(triggerVariable);
} catch (PlcInvalidFieldException e) {
logger.debug(e.getMessage(), e);
LOGGER.debug(e.getMessage(), e);
String exceptionMessage = String.format("Invalid trigger Field for Job %s: %s", triggeredScrapeJobImpl.getJobName(), triggerVariable);
throw new ScraperException(exceptionMessage);
}
Expand Down Expand Up @@ -168,9 +171,9 @@ boolean evaluateTrigger(Object value) throws ScraperException {

switch (this.comparatorType) {
case EQUAL:
return currentValue == refValue;
return isApproximately(currentValue,refValue, TOLERANCE_FLOATING_EQUALITY);
case UNEQUAL:
return currentValue != refValue;
return !isApproximately(currentValue,refValue, TOLERANCE_FLOATING_EQUALITY);
case SMALLER:
return currentValue < refValue;
case SMALLER_EQUAL:
Expand Down Expand Up @@ -284,6 +287,7 @@ private Object convertCompareValue(String compareValue) throws ScraperException
return Double.parseDouble(compareValue);
}
catch (Exception e){
LOGGER.debug(e.getMessage(), e);
String exceptionMessage = String.format("No valid compare Value at DataType Numeric for trigger for Job %s: %s",triggeredScrapeJobImpl.getJobName(),compareValue);
throw new ScraperException(exceptionMessage);
}
Expand All @@ -306,7 +310,7 @@ public static TriggerConfiguration createConfiguration(String jobTriggerStrategy
String strat = matcher.group("strategy");
String scheduledMs = matcher.group("scheduledInterval");

logger.debug("Strategy: {}, scheduled ms: {}",strat,scheduledMs);
LOGGER.debug("Strategy: {}, scheduled ms: {}",strat,scheduledMs);

String triggerVar = matcher.group("triggerVar");
String comparatorString = matcher.group("comp");
Expand Down Expand Up @@ -334,7 +338,7 @@ public static TriggerConfiguration createConfiguration(String jobTriggerStrategy

private void handleException(Exception e){
//push up if needed
logger.debug("Exception: ", e);
LOGGER.debug("Exception: ", e);
}

TriggerType getTriggerType() {
Expand All @@ -357,6 +361,18 @@ Object getCompareValue() {
return compareValue;
}

/**
* check for approximate equality to avoid "Floating-point expressions shall not be tested for equality or inequality." Sonar-Bug
* @param self current value
* @param other reference value
* @param within tolerance band
* @return if approximate equal, false otherwise
*/
private static boolean isApproximately(double self, double other, double within)
{
return Math.abs(self - other) <= within;
}

public enum Comparators{
EQUAL,
UNEQUAL,
Expand Down
7 changes: 4 additions & 3 deletions ...in/java/org/apache/plc4x/java/scraper/triggeredscraper/triggerhandler/TriggerHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
* holds the handler for the regarding trigger-scraper on rising-trigger edge
*/
public class TriggerHandler {
private static final Logger logger = LoggerFactory.getLogger(TriggerHandler.class);
private static final Logger LOGGER = LoggerFactory.getLogger(TriggerHandler.class);
private static final String TRIGGER = "trigger";

private final TriggerConfiguration triggerConfiguration;
Expand Down Expand Up @@ -78,14 +78,14 @@ private boolean checkS7TriggerVariable(){
try {
return parentScraperTask.getDriverManager().getConnection(parentScraperTask.getConnectionString());
} catch (PlcConnectionException e) {
logger.warn("Unable to instantiate connection to " + parentScraperTask.getConnectionString(), e);
LOGGER.warn("Unable to instantiate connection to " + parentScraperTask.getConnectionString(), e);
throw new PlcRuntimeException(e);
}
}, parentScraperTask.getExecutorService());
PlcConnection connection = null;
try {
connection = future.get(parentScraperTask.getRequestTimeoutMs(), TimeUnit.MILLISECONDS);
logger.trace("Connection to {} established: {}", parentScraperTask.getConnectionString(), connection);
LOGGER.trace("Connection to {} established: {}", parentScraperTask.getConnectionString(), connection);
PlcReadRequest.Builder builder = connection.readRequestBuilder();
builder.addItem(TRIGGER, triggerConfiguration.getTriggerVariable());
PlcReadResponse response = builder
Expand Down Expand Up @@ -116,6 +116,7 @@ private boolean checkS7TriggerVariable(){
try {
connection.close();
} catch (Exception e) {
LOGGER.warn("Error on closing connection",e);
// intentionally do nothing
}
}
Expand Down

0 comments on commit de8877f

Please sign in to comment.