Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added logging based on slf4j #22

Merged
merged 1 commit into from
Jun 18, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ocpp-common/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@
<version>21.0</version>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
<version>1.7.10</version>
<scope>compile</scope>
</dependency>
</dependencies>

<build>
Expand Down
7 changes: 6 additions & 1 deletion ocpp-common/src/main/java/eu/chargetime/ocpp/Client.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
import eu.chargetime.ocpp.model.Request;

import java.util.concurrent.CompletableFuture;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/*
ChargeTime.eu - Java-OCA-OCPP
Copyright (C) 2015-2016 Thomas Volden <[email protected]>
Expand Down Expand Up @@ -44,6 +47,8 @@ of this software and associated documentation files (the "Software"), to deal
*/
public abstract class Client extends FeatureHandler
{
private static final Logger logger = LoggerFactory.getLogger(Client.class);

private Session session;

/**
Expand Down Expand Up @@ -115,7 +120,7 @@ public void disconnect()
try {
session.close();
} catch (Exception ex) {
ex.printStackTrace();
logger.info("session.close() failed", ex);
}
}

Expand Down
17 changes: 11 additions & 6 deletions ocpp-common/src/main/java/eu/chargetime/ocpp/Communicator.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

import java.util.ArrayDeque;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/*
ChargeTime.eu - Java-OCA-OCPP
Copyright (C) 2015-2016 Thomas Volden <[email protected]>
Expand Down Expand Up @@ -41,7 +44,9 @@ of this software and associated documentation files (the "Software"), to deal
* Must be overloaded to implement a specific format.
*/
public abstract class Communicator {
private RetryRunner retryRunner;
private static final Logger logger = LoggerFactory.getLogger(Communicator.class);

private RetryRunner retryRunner;
protected Radio radio;
private ArrayDeque<Object> transactionQueue;
private CommunicatorEvents events;
Expand Down Expand Up @@ -158,7 +163,7 @@ synchronized public void sendCall(String uniqueId, String action, Request reques
radio.send(call);
}
} catch (NotConnectedException ex) {
ex.printStackTrace();
logger.warn("sendCall() failed", ex);
if (request.transactionRelated()) {
transactionQueue.add(call);
} else {
Expand All @@ -176,8 +181,8 @@ synchronized public void sendCall(String uniqueId, String action, Request reques
public void sendCallResult(String uniqueId, String action, Confirmation confirmation) {
try {
radio.send(makeCallResult(uniqueId, action, packPayload(confirmation)));
} catch (NotConnectedException e) {
e.printStackTrace();
} catch (NotConnectedException ex) {
logger.warn("sendCallResult() failed", ex);
events.onError(uniqueId, "Not connected", "The confirmation couldn't be send due to the lack of connection", confirmation);
}
}
Expand All @@ -193,7 +198,7 @@ public void sendCallError(String uniqueId, String action, String errorCode, Stri
try {
radio.send(makeCallError(uniqueId, action, errorCode, errorDescription));
} catch (NotConnectedException ex) {
ex.printStackTrace();
logger.warn("sendCallError() failed", ex);
events.onError(uniqueId, "Not connected", "The error couldn't be send due to the lack of connection", errorCode);
}
}
Expand Down Expand Up @@ -292,7 +297,7 @@ public void run() {
popRetryMessage();
}
} catch (Exception ex) {
ex.printStackTrace();
logger.warn("RetryRunner::run() failed", ex);
}
}
}
Expand Down
11 changes: 9 additions & 2 deletions ocpp-common/src/main/java/eu/chargetime/ocpp/Queue.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
import java.util.HashMap;
import java.util.UUID;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/*
ChargeTime.eu - Java-OCA-OCPP
Copyright (C) 2015-2016 Thomas Volden <[email protected]>
Expand Down Expand Up @@ -37,7 +40,9 @@ of this software and associated documentation files (the "Software"), to deal
*/
public class Queue
{
private HashMap<String, Request> requestQueue;
private static final Logger logger = LoggerFactory.getLogger(Queue.class);

private HashMap<String, Request> requestQueue;

public Queue () {
requestQueue = new HashMap<>();
Expand All @@ -59,6 +64,8 @@ public String store(Request request) {
* Restore a {@link Request} using a unique identifier.
* The identifier can only be used once.
* If no Request was found, null is returned.
*
* FIXME: use optional instead
*
* @param ticket unique identifier returned when {@link Request} was initially stored.
* @return the stored {@link Request}
Expand All @@ -69,7 +76,7 @@ public Request restoreRequest(String ticket) {
request = requestQueue.get(ticket);
requestQueue.remove(ticket);
} catch (Exception ex) {
ex.printStackTrace();
logger.warn("restoreRequest({}) failed", ticket, ex);
}
return request;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,12 @@ of this software and associated documentation files (the "Software"), to deal

import java.util.concurrent.CompletableFuture;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

abstract class RequestDispatcher {

protected SessionEvents eventHandler;
protected SessionEvents eventHandler;

public CompletableFuture<Confirmation> handleRequest(Request request)
{
Expand All @@ -40,6 +43,7 @@ public CompletableFuture<Confirmation> handleRequest(Request request)
return promise;
}

// FIXME: fix typo fulfillPromis -> fulfillPromise
protected abstract void fulfillPromis(CompletableFuture<Confirmation> promise, Request request);

public void setEventHandler(SessionEvents eventHandler) {
Expand All @@ -58,14 +62,15 @@ protected void fulfillPromis(CompletableFuture<Confirmation> promise, Request re
}

class SimpleRequestDispatcher extends RequestDispatcher {
private static final Logger logger = LoggerFactory.getLogger(SimpleRequestDispatcher.class);

@Override
protected void fulfillPromis(CompletableFuture<Confirmation> promise, Request request) {
try {
Confirmation conf = eventHandler.handleRequest(request);
promise.complete(conf);
} catch (Exception ex) {
ex.printStackTrace();
logger.warn("fulfillPromis() failed", ex);
promise.completeExceptionally(ex);
}
}
Expand Down
38 changes: 24 additions & 14 deletions ocpp-common/src/main/java/eu/chargetime/ocpp/Session.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

import java.util.concurrent.CompletableFuture;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/*
ChargeTime.eu - Java-OCA-OCPP
Copyright (C) 2015-2016 Thomas Volden <[email protected]>
Expand Down Expand Up @@ -39,6 +42,8 @@ of this software and associated documentation files (the "Software"), to deal
*/
public class Session {

private static final Logger logger = LoggerFactory.getLogger(Session.class);

private Communicator communicator;
private Queue queue;
private RequestDispatcher dispatcher;
Expand Down Expand Up @@ -119,26 +124,31 @@ public void accept(SessionEvents eventHandler) {
}

private class CommunicatorEventHandler implements CommunicatorEvents {
private static final String OCCURENCE_CONSTRAINT_VIOLATION = "Payload for Action is syntactically correct but at least one of the fields violates occurence constraints";
private static final String FIELD_CONSTRAINT_VIOLATION = "Field %s violates constraints with value: \"%s\". %s";
private static final String INTERNAL_ERROR = "An internal error occurred and the receiver was not able to process the requested Action successfully";
private static final String UNABLE_TO_PROCESS = "Unable to process action";

@Override
public void onCallResult(String id, String action, Object payload) {
try {
Confirmation confirmation = communicator.unpackPayload(payload, getConfirmationType(id));
if (confirmation.validate()) {
events.handleConfirmation(id, confirmation);
} else {
communicator.sendCallError(id, action, "OccurenceConstraintViolation", "Payload for Action is syntactically correct but at least one of the fields violates occurence constraints");
communicator.sendCallError(id, action, "OccurenceConstraintViolation", OCCURENCE_CONSTRAINT_VIOLATION);
}
}
catch (PropertyConstraintException ex) {
String message = "Field %s violates constraints with value: \"%s\". %s";
communicator.sendCallError(id, action, "TypeConstraintViolation", String.format(message, ex.getFieldKey(), ex.getFieldValue(), ex.getMessage()));
ex.printStackTrace();
String message = String.format(FIELD_CONSTRAINT_VIOLATION, ex.getFieldKey(), ex.getFieldValue(), ex.getMessage());
logger.warn(message, ex);
communicator.sendCallError(id, action, "TypeConstraintViolation", message);
} catch (UnsupportedFeatureException ex) {
communicator.sendCallError(id, action, "InternalError", "An internal error occurred and the receiver was not able to process the requested Action successfully");
ex.printStackTrace();
logger.warn(INTERNAL_ERROR, ex);
communicator.sendCallError(id, action, "InternalError", INTERNAL_ERROR);
} catch (Exception ex) {
communicator.sendCallError(id, action, "FormationViolation", "Unable to process action");
ex.printStackTrace();
logger.warn(UNABLE_TO_PROCESS, ex);
communicator.sendCallError(id, action, "FormationViolation", UNABLE_TO_PROCESS);
}
}

Expand All @@ -154,15 +164,15 @@ synchronized public void onCall(String id, String action, Object payload) {
CompletableFuture<Confirmation> promise = dispatcher.handleRequest(request);
promise.whenComplete(new ConfirmationHandler(id, action, communicator));
} else {
communicator.sendCallError(id, action, "OccurenceConstraintViolation", "Payload for Action is syntactically correct but at least one of the fields violates occurence constraints");
communicator.sendCallError(id, action, "OccurenceConstraintViolation", OCCURENCE_CONSTRAINT_VIOLATION);
}
} catch (PropertyConstraintException ex) {
ex.printStackTrace();
String message = "Field %s violates constraints with value: \"%s\". %s";
communicator.sendCallError(id, action, "TypeConstraintViolation", String.format(message, ex.getFieldKey(), ex.getFieldValue(), ex.getMessage()));
String message = String.format(FIELD_CONSTRAINT_VIOLATION, ex.getFieldKey(), ex.getFieldValue(), ex.getMessage());
logger.warn(message, ex);
communicator.sendCallError(id, action, "TypeConstraintViolation", message);
} catch (Exception ex) {
ex.printStackTrace();
communicator.sendCallError(id, action, "FormationViolation", "Unable to process action");
logger.warn(UNABLE_TO_PROCESS, ex);
communicator.sendCallError(id, action, "FormationViolation", UNABLE_TO_PROCESS);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ public abstract class ModelUtil {
* @param hayStack list of value that we search in.
* @return true if value was found in list.
*/
public static boolean isAmong(Object needle, Object... hayStack) {
public static boolean isAmong(String needle, String... hayStack) {
boolean found = false;
if (hayStack != null) {
for (Object straw : hayStack) {
for (String straw : hayStack) {
if (found = isNullOrEqual(straw, needle)) {
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,18 @@ public void isAmong_haystackIsNull_returnsFalse() {
@Test
public void isAmong_needleAndHaystackIsNull_returnsFalse() {
// When
boolean found = ModelUtil.isAmong(null, null);
boolean found = ModelUtil.isAmong((String)null, (String)null);

// Then
assertThat(found, is(false));
// FIXME: please check if thats intended! I think the behaviour should be identical to the version below!
// assertThat(found, is(false));
assertThat(found, is(true));
}

@Test
public void isAmong_needleIsNullAndHaystackIsListWithNulls_returnsTrue() {
// When
boolean found = ModelUtil.isAmong(null, null, null);
boolean found = ModelUtil.isAmong((String)null, (String)null, (String)null);

// Then
assertThat(found, is(true));
Expand Down
18 changes: 18 additions & 0 deletions ocpp-v1_6-test/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,24 @@
<version>1.3</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-core</artifactId>
<version>1.1.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
<version>1.1.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>jul-to-slf4j</artifactId>
<version>1.7.10</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
14 changes: 9 additions & 5 deletions ocpp-v1_6/src/main/java/eu/chargetime/ocpp/SOAPClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ of this software and associated documentation files (the "Software"), to deal
import eu.chargetime.ocpp.model.SOAPHostInfo;

import javax.xml.soap.SOAPMessage;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.URL;
Expand All @@ -38,8 +42,8 @@ of this software and associated documentation files (the "Software"), to deal
import java.util.concurrent.Executors;

public class SOAPClient extends Client {

final private String WSDL_CHARGE_POINT = "eu/chargetime/ocpp/OCPP_ChargePointService_1.6.wsdl";
private static final Logger logger = LoggerFactory.getLogger(SOAPClient.class);
private static final String WSDL_CHARGE_POINT = "eu/chargetime/ocpp/OCPP_ChargePointService_1.6.wsdl";

private SOAPCommunicator communicator;
private WebServiceTransmitter transmitter;
Expand Down Expand Up @@ -122,17 +126,17 @@ private void openWS() {
try {
soapMessage = transmitter.relay(message.getMessage()).get();
} catch (InterruptedException e) {
//e.printStackTrace();
logger.warn("openWS() transmitter.relay failed", e);
} catch (ExecutionException e) {
e.printStackTrace();
logger.warn("openWS() transmitter.relay failed", e);
}
return soapMessage;
}));
threadPool = Executors.newCachedThreadPool();
server.setExecutor(threadPool);
server.start();
} catch (IOException e) {
e.printStackTrace();
logger.warn("openWS() failed", e);
}
}

Expand Down
Loading