jsinovassin commented on code in PR #696:
URL: https://github.com/apache/unomi/pull/696#discussion_r1775518742
##########
extensions/router/router-core/src/main/java/org/apache/unomi/router/core/event/UpdateCamelRouteEventHandler.java:
##########
@@ -30,23 +30,23 @@
* @author dgaillard
*/
public class UpdateCamelRouteEventHandler extends CellarSupport implements
EventHandler<UpdateCamelRouteEvent> {
- private static final Logger logger =
LoggerFactory.getLogger(UpdateCamelRouteEventHandler.class.getName());
+ private static final Logger LLOGGER =
LoggerFactory.getLogger(UpdateCamelRouteEventHandler.class.getName());
Review Comment:
Small typo here
##########
itests/src/test/java/org/apache/unomi/itests/BaseIT.java:
##########
@@ -546,7 +547,9 @@ protected CloseableHttpResponse
executeHttpRequest(HttpUriRequest request) throw
protected String resourceAsString(final String resource) {
final java.net.URL url =
bundleContext.getBundle().getResource(resource);
try (InputStream stream = url.openStream()) {
- return
objectMapper.writeValueAsString(objectMapper.readTree(stream));
+ JsonNode node = objectMapper.readTree(stream);
Review Comment:
Why did you need to declare local variable here?
##########
plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluator.java:
##########
@@ -348,11 +344,8 @@ protected Object getOGNLPropertyValue(Item item, String
expression) throws Excep
try {
return accessor.get(ognlContext, item);
} catch (Throwable t) {
- logger.error("Error evaluating expression on item {}. See
debug level for more information", item.getClass().getName());
- if (logger.isDebugEnabled()) {
- logger.debug("Error evaluating expression {} on item {}.",
expression, item.getClass().getName(), t);
- }
-
+ LOGGER.error("Error evaluating expression on item {}. See
debug level for more information", item.getClass().getName());
+ if (LOGGER.isDebugEnabled()) LOGGER.debug("Error evaluating
expression {} on item {}.", expression, item.getClass().getName(), t);
Review Comment:
The condition could be removed here as well
##########
services/src/main/java/org/apache/unomi/services/impl/goals/GoalsServiceImpl.java:
##########
@@ -78,7 +78,7 @@ public void setRulesService(RulesService rulesService) {
}
public void postConstruct() {
- logger.debug("postConstruct {" + bundleContext.getBundle() + "}");
+ LOGGER.debug("postConstruct {" + bundleContext.getBundle() + "}");
Review Comment:
This log line could be changed as the other logs
##########
services/src/main/java/org/apache/unomi/services/impl/cluster/ClusterSystemStatisticsEventHandler.java:
##########
@@ -39,7 +39,7 @@
public class ClusterSystemStatisticsEventHandler extends CellarSupport
implements EventHandler<ClusterSystemStatisticsEvent> {
public static final String SWITCH_ID =
"org.apache.unomi.cluster.system.statistics.handler";
- private static final Logger logger =
LoggerFactory.getLogger(ClusterSystemStatisticsEventHandler.class.getName());
+ private static final Logger LLOGGER =
LoggerFactory.getLogger(ClusterSystemStatisticsEventHandler.class.getName());
Review Comment:
Small typo here
##########
plugins/request/src/test/java/org/apache/unomi/plugins/request/actions/UserAgentDetectorTest.java:
##########
@@ -82,7 +82,7 @@ private void executeWorker(ExecutorService executorService,
int workerCount) thr
}
executorService.invokeAll(callables);
long totalTime = System.currentTimeMillis() - startTime;
- logger.info("AgentWorker workers completed execution in " + totalTime
+ "ms");
+ LOGGER.info("AgentWorker workers completed execution in {}" +
totalTime + "ms");
Review Comment:
This modification seems to be unnecessary
##########
plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluator.java:
##########
@@ -196,10 +196,8 @@ public boolean eval(Condition condition, Item item,
Map<String, Object> context,
if (!(e instanceof OgnlException)
|| (!StringUtils.startsWith(e.getMessage(),
"source is null for getProperty(null"))) {
- logger.warn("Error evaluating value for {} {}. See debug
level for more information", item.getClass().getName(), name);
- if (logger.isDebugEnabled()) {
- logger.debug("Error evaluating value for {} {}",
item.getClass().getName(), name, e);
- }
+ LOGGER.warn("Error evaluating value for {} {}. See debug
level for more information", item.getClass().getName(), name);
+ if (LOGGER.isDebugEnabled()) LOGGER.debug("Error
evaluating value for {} {}", item.getClass().getName(), name, e);
Review Comment:
I think this condition can be removed as well
##########
rest/src/main/java/org/apache/unomi/rest/exception/ValidationExceptionMapper.java:
##########
@@ -29,17 +29,13 @@
@Provider
@Component(service = ExceptionMapper.class)
public class ValidationExceptionMapper implements
ExceptionMapper<ConstraintViolationException> {
- private static final Logger logger =
LoggerFactory.getLogger(ValidationExceptionMapper.class.getName());
+ private static final Logger LOGGER =
LoggerFactory.getLogger(ValidationExceptionMapper.class.getName());
@Override
public Response toResponse(ConstraintViolationException exception) {
exception.getConstraintViolations().forEach(constraintViolation -> {
- if (logger.isDebugEnabled()) {
- logger.debug(String.format("value %s from %s %s",
constraintViolation.getInvalidValue(),
- constraintViolation.getPropertyPath().toString(),
constraintViolation.getMessage()), exception);
- }
- logger.error(constraintViolation.getPropertyPath().toString() + "
" + constraintViolation.getMessage() + ". Enable debug log "
- + "level for more informations about the invalid value
received");
+ LOGGER.error("{} {}. {}", constraintViolation.getPropertyPath(),
constraintViolation.getMessage(), (LOGGER.isDebugEnabled())?"":"Enable debug
log level for more information about the invalid value received");
+ if (LOGGER.isDebugEnabled()) LOGGER.debug("value {} from {} {}",
constraintViolation.getInvalidValue(), constraintViolation.getPropertyPath(),
constraintViolation.getMessage(), exception);
Review Comment:
This condition could be removed as well
##########
plugins/request/src/test/java/org/apache/unomi/plugins/request/actions/UserAgentDetectorTest.java:
##########
@@ -34,7 +34,7 @@
@FixMethodOrder(MethodSorters.NAME_ASCENDING)
public class UserAgentDetectorTest {
- private static final Logger logger =
Logger.getLogger(UserAgentDetectorTest.class.getName());
+ private static final Logger LOGGER =
Logger.getLogger(UserAgentDetectorTest.class.getName());
Review Comment:
This class is using java.util.logging.Logger instead of org.slf4j.Logger
don't we want to change it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
