Cleaned up logging for NSI.

subprojects/replication/src/main/java/com/trifork/stamdata/replication/ApplicationContextListener.java

@@ -17,7 +17,7 @@
 
 package com.trifork.stamdata.replication;
 
-import static org.slf4j.LoggerFactory.*;
+import static org.slf4j.LoggerFactory.getLogger;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -31,6 +31,7 @@
 import com.google.inject.servlet.GuiceServletContextListener;
 import com.trifork.stamdata.replication.db.DatabaseModule;
 import com.trifork.stamdata.replication.gui.GuiModule;
+import com.trifork.stamdata.replication.logging.LoggingModule;
 import com.trifork.stamdata.replication.monitoring.MonitoringModule;
 import com.trifork.stamdata.replication.monitoring.ProfilingModule;
 import com.trifork.stamdata.replication.replication.RegistryModule;

subprojects/replication/src/main/java/com/trifork/stamdata/replication/gui/controllers/ClientController.java

@@ -33,9 +33,9 @@
 import com.trifork.stamdata.replication.gui.models.Client;
 import com.trifork.stamdata.replication.gui.models.ClientDao;
 import com.trifork.stamdata.replication.gui.models.User;
+import com.trifork.stamdata.replication.logging.AuditLogger;
 import com.trifork.stamdata.replication.replication.annotations.Registry;
 import com.trifork.stamdata.replication.replication.views.View;
-import com.trifork.stamdata.replication.util.DatabaseAuditLogger;
 
 
 @Singleton
@@ -53,7 +53,7 @@ public class ClientController extends AbstractController {
 	private final Provider<User> user;
 
 	@Inject
-	public ClientController(Provider<User> user, Provider<ClientDao> clients, Provider<PageRenderer> renderer, Provider<DatabaseAuditLogger> audit, @Registry Map<String, Class<? extends View>> registry) {
+	public ClientController(Provider<User> user, Provider<ClientDao> clients, Provider<PageRenderer> renderer, Provider<AuditLogger> audit, @Registry Map<String, Class<? extends View>> registry) {
 
 		this.user = checkNotNull(user);
 		this.clients = checkNotNull(clients);
@@ -112,7 +112,7 @@ protected void create(HttpServletRequest request, HttpServletResponse response)
 		Client client = clients.get().create(name, cvr);
 
 		if (client != null) {
-			audit.get().write("New client %s created by %s.", client, user.get());
+			audit.get().log("New client %s created by %s.", client, user.get());
 		}
 
 		redirect(request, response, "/admin/clients");
@@ -140,7 +140,12 @@ protected void getUpdate(HttpServletRequest request, HttpServletResponse respons
 		Client client = clients.get().find(id);
 
 		for (String view : views)
+
+		AuditLogger auditLogger = audit.get();
+
 			client.addPermission(view);
+			auditLogger.log("User=%s granted Client=%s access to View=%s", user.get(), client, view);
+			auditLogger.log("User=%s restricted Client=%s from accessing View=%s", user.get(), client, view);
 
 		clients.get().update(client);
 
@@ -154,7 +159,7 @@ protected void getDelete(HttpServletRequest request, HttpServletResponse respons
 		Client client = clients.get().find(id);
 
 		if (clients.get().delete(id)) {
-			audit.get().write("Client %s was deleted by %s.", client, user.get());
+			audit.get().log("Client %s was deleted by %s.", client, user.get());
 		}
 
 		redirect(request, response, "/admin/clients");

subprojects/replication/src/main/java/com/trifork/stamdata/replication/gui/controllers/LogController.java

@@ -27,7 +27,7 @@
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
-import com.trifork.stamdata.replication.util.DatabaseAuditLogger;
+import com.trifork.stamdata.replication.logging.DatabaseAuditLogger;
 
 
 @Singleton

subprojects/replication/src/main/java/com/trifork/stamdata/replication/gui/controllers/UserController.java

@@ -29,7 +29,7 @@
 import com.trifork.stamdata.replication.gui.annotations.Whitelist;
 import com.trifork.stamdata.replication.gui.models.User;
 import com.trifork.stamdata.replication.gui.models.UserDao;
-import com.trifork.stamdata.replication.util.DatabaseAuditLogger;
+import com.trifork.stamdata.replication.logging.AuditLogger;
 
 
 @Singleton
@@ -39,13 +39,13 @@ public class UserController extends AbstractController {
 
 	private final Provider<UserDao> users;
 	private final Map<String, String> whitelist;
-	private final Provider<DatabaseAuditLogger> audit;
+	private final Provider<AuditLogger> audit;
 	private final Provider<PageRenderer> renderer;
 
 	private final Provider<User> currentUser;
 
 	@Inject
-	UserController(Provider<User> currentUser, @Whitelist Map<String, String> whitelist, Provider<UserDao> users, Provider<DatabaseAuditLogger> audit, Provider<PageRenderer> renderer) {
+	UserController(Provider<User> currentUser, @Whitelist Map<String, String> whitelist, Provider<UserDao> users, Provider<AuditLogger> audit, Provider<PageRenderer> renderer) {
 
 		this.currentUser = currentUser;
 		this.whitelist = whitelist;
@@ -109,7 +109,7 @@ protected void getCreate(HttpServletRequest request, HttpServletResponse respons
 			User user = users.get().create(newUserName, newUserCPR, newUserCVR);
 
 			if (user != null) {
-				audit.get().write("New administrator created (new_user_cpr=%s, new_user_cvr=%s). Created by user_cpr=%s.", newUserCPR, newUserCVR, currentUser.get().getCpr());
+				audit.get().log("new_user=%s created by user=%s.", user, currentUser.get());
 			}
 		}
 
@@ -125,7 +125,7 @@ protected void getDelete(HttpServletRequest request, HttpServletResponse respons
 		if (deletedUser != null) {
 			String userCPR = currentUser.get().getCpr();
 			users.get().delete(id);
-			audit.get().write("Administrator '%s (ID=%s)' was deleted by user %s.", deletedUser.getName(), deletedUser.getId(), userCPR);
+			audit.get().log("user=%s, deleted_user=%s", user, deletedUser);
 		}
 
 		redirect(request, response, "/admin/users");

subprojects/replication/src/main/java/com/trifork/stamdata/replication/logging/AuditLogger.java

@@ -0,0 +1,19 @@
+package com.trifork.stamdata.replication.logging;
+
+/**
+ * An abstract for different kinds of audit loggers.
+ * 
+ * Uses the Strategy Pattern.
+ * 
+ * @author Thomas Børlum (thb@trifork.com)
+ */
+public interface AuditLogger {
+
+	/**
+	 * Logs a message formed by the format and arguments.
+	 * 
+	 * @param format The format of the message.
+	 * @param args The arguments to be inserted into the format.
+	 */
+	void log(String format, Object...args);
+}

subprojects/replication/src/main/java/com/trifork/stamdata/replication/logging/CompositeAuditLogger.java

@@ -0,0 +1,31 @@
+package com.trifork.stamdata.replication.logging;
+
+import com.google.inject.Inject;
+
+/**
+ * Facade that logs to both file and database.
+ * 
+ * @see SLF4JAuditLogger
+ * @see DatabaseAuditLogger
+ * 
+ * @author Thomas Børlum (thb@trifork.com)
+ */
+public class CompositeAuditLogger implements AuditLogger {
+
+	private final DatabaseAuditLogger dbLogger;
+	private final SLF4JAuditLogger fileLogger;
+
+	@Inject
+	CompositeAuditLogger(DatabaseAuditLogger dbLogger, SLF4JAuditLogger fileLogger) {
+
+		this.dbLogger = dbLogger;
+		this.fileLogger = fileLogger;
+	}
+
+	@Override
+	public void log(String format, Object... args) {
+
+		dbLogger.log(format, args);
+		fileLogger.log(format, args);
+	}
+}

subprojects/replication/src/main/java/com/trifork/stamdata/replication/util/DatabaseAuditLogger.java → subprojects/replication/src/main/java/com/trifork/stamdata/replication/logging/DatabaseAuditLogger.java

@@ -15,7 +15,7 @@
 // of which can be found at the link below.
 // http://www.gnu.org/copyleft/lesser.html
 
-package com.trifork.stamdata.replication.util;
+package com.trifork.stamdata.replication.logging;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static java.lang.String.format;
@@ -27,8 +27,16 @@
 
 import com.google.inject.Inject;
 
-
-public class DatabaseAuditLogger {
+/**
+ * Logs audit messages to the database.
+ * 
+ * This class is also used as a data access class to the log entries.
+ * 
+ * @see LogController
+ * 
+ * @author Thomas Børlum (thb@trifork.com)
+ */
+public class DatabaseAuditLogger implements AuditLogger {
 
 	private final Session em;
 
@@ -50,15 +58,14 @@ public List<LogEntry> findAll(int offset, int count) {
 		return query.list();
 	}
 
-	public boolean write(String message, Object... args) {
+	public void log(String message, Object... args) {
 
-		return write(format(message, args));
+		write(format(message, args));
 	}
 
-	public boolean write(String message) {
+	public void write(String message) {
 
 		LogEntry entry = new LogEntry(message);
 		em.persist(entry);
-		return true;
 	}
 }

subprojects/replication/src/main/java/com/trifork/stamdata/replication/util/LogEntry.java → subprojects/replication/src/main/java/com/trifork/stamdata/replication/logging/LogEntry.java

@@ -15,7 +15,7 @@
 // of which can be found at the link below.
 // http://www.gnu.org/copyleft/lesser.html
 
-package com.trifork.stamdata.replication.util;
+package com.trifork.stamdata.replication.logging;
 
 import static javax.persistence.TemporalType.TIMESTAMP;
 import java.util.Date;

subprojects/replication/src/main/java/com/trifork/stamdata/replication/logging/LoggingModule.java

@@ -0,0 +1,22 @@
+package com.trifork.stamdata.replication.logging;
+
+import com.google.inject.Module;
+import com.google.inject.PrivateModule;
+
+
+public class LoggingModule extends PrivateModule implements Module {
+
+	@Override
+	protected void configure() {
+
+		bind(DatabaseAuditLogger.class);
+		bind(SLF4JAuditLogger.class);
+		bind(AuditLogger.class).to(CompositeAuditLogger.class);
+		expose(AuditLogger.class);
+
+		// The Database logger is exposed too since it is needed
+		// in the LogController, for historical reasons.
+
+		expose(DatabaseAuditLogger.class);
+	}
+}

subprojects/replication/src/main/java/com/trifork/stamdata/replication/logging/SLF4JAuditLogger.java

@@ -0,0 +1,20 @@
+package com.trifork.stamdata.replication.logging;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Logs audit messages using to a file using SLF4J.
+ * 
+ * @author Thomas Børlum (thb@trifork.com)
+ */
+public class SLF4JAuditLogger implements AuditLogger {
+
+	private static final Logger logger = LoggerFactory.getLogger(AuditLogger.class);
+
+	@Override
+	public void log(String format, Object... args) {
+
+		logger.info(String.format(format, args));
+	}
+}

subprojects/replication/src/main/java/com/trifork/stamdata/replication/security/dgws/AuthorizationServlet.java

@@ -41,10 +41,11 @@
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
+import com.trifork.stamdata.replication.logging.AuditLogger;
 import com.trifork.stamdata.replication.replication.annotations.Registry;
 import com.trifork.stamdata.replication.replication.views.View;
 import com.trifork.stamdata.replication.replication.views.Views;
-import com.trifork.stamdata.replication.util.DatabaseAuditLogger;
+
 import dk.sosi.seal.SOSIFactory;
 import dk.sosi.seal.model.IDCard;
 import dk.sosi.seal.model.Reply;
@@ -134,13 +135,13 @@ static class RequestProcessor {
 
 		private final Map<String, Class<? extends View>> registry;
 		private final DGWSSecurityManager securityManager;
-		private final DatabaseAuditLogger audit;
+		private final AuditLogger audit;
 		private final SOSIFactory factory;
 		private final Marshaller marshaller;
 		private final Unmarshaller unmarshaller;
 
 		@Inject
-		RequestProcessor(SOSIFactory factory, DatabaseAuditLogger audit, DGWSSecurityManager securityManager, @Registry Map<String, Class<? extends View>> registry, Marshaller marshaller, Unmarshaller unmarshaller) throws JAXBException {
+		RequestProcessor(SOSIFactory factory, AuditLogger audit, DGWSSecurityManager securityManager, @Registry Map<String, Class<? extends View>> registry, Marshaller marshaller, Unmarshaller unmarshaller) throws JAXBException {
 
 			this.marshaller = checkNotNull(marshaller);
 			this.unmarshaller = checkNotNull(unmarshaller);
@@ -209,7 +210,7 @@ public Reply process(Request request) {
 				// TODO: Somehow we should consolidate the audit log and
 				// the error log.
 
-				audit.write("Access request: clientCvr=%s, entityUri=%s", cvr, requestBody.getViewURI());
+				audit.log("Access request: clientCvr=%s, entityUri=%s", cvr, requestBody.getViewURI());
 
 				// CHECK THAT THE REQUESTED VIEW EXISTS
 				//

subprojects/replication/src/test/java/com/trifork/stamdata/replication/security/dgws/RequestProcessorTest.java

@@ -37,10 +37,11 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.w3c.dom.Node;
+
+import com.trifork.stamdata.replication.logging.DatabaseAuditLogger;
 import com.trifork.stamdata.replication.replication.views.MockView;
 import com.trifork.stamdata.replication.replication.views.View;
 import com.trifork.stamdata.replication.security.dgws.AuthorizationServlet.RequestProcessor;
-import com.trifork.stamdata.replication.util.DatabaseAuditLogger;
 import dk.sosi.seal.SOSIFactory;
 import dk.sosi.seal.model.CareProvider;
 import dk.sosi.seal.model.IDCard;