From 74a649d2d76b7a7d9d26ff449a9c41c5220905be Mon Sep 17 00:00:00 2001
From: Robert von Burg <eitch@eitchnet.ch>
Date: Mon, 1 Apr 2019 16:53:07 +0200
Subject: [PATCH] [Fix] Fixed NPE in DefaultServiceHandler

---
 .../strolch/service/api/AbstractService.java  | 18 +++++++++++++++
 .../service/api/DefaultServiceHandler.java    | 23 +++++++++++++++----
 .../test/java/li/strolch/service/TxTest.java  |  2 +-
 .../li/strolch/service/test/ServiceTest.java  |  7 ------
 4 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/li.strolch.agent/src/main/java/li/strolch/service/api/AbstractService.java b/li.strolch.agent/src/main/java/li/strolch/service/api/AbstractService.java
index fbf7cae25..5a1182202 100644
--- a/li.strolch.agent/src/main/java/li/strolch/service/api/AbstractService.java
+++ b/li.strolch.agent/src/main/java/li/strolch/service/api/AbstractService.java
@@ -163,6 +163,24 @@ public abstract class AbstractService<T extends ServiceArgument, U extends Servi
 		return this.container.getRealm(realm).openTx(getCertificate(), getClass(), false);
 	}
 
+	/**
+	 * Opens a {@link StrolchTransaction} for the given realm, the action for the TX is this implementation's class
+	 * name. This transaction should be used in a try-with-resource clause so it is properly closed
+	 *
+	 * @param realm
+	 * 		the name of the realm to return
+	 * @param readOnly
+	 * 		true if this TX is so be read-only
+	 *
+	 * @return the open {@link StrolchTransaction}
+	 *
+	 * @throws StrolchException
+	 * 		if the {@link StrolchRealm} does not exist with the given name
+	 */
+	protected StrolchTransaction openTx(String realm, boolean readOnly) throws StrolchException {
+		return this.container.getRealm(realm).openTx(getCertificate(), getClass(), readOnly);
+	}
+
 	/**
 	 * Opens a {@link StrolchTransaction} by evaluating if the given argument has a realm defined, if not, then the
 	 * realm from the user certificate is used. The action for the TX is this implementation's class name. This
diff --git a/li.strolch.agent/src/main/java/li/strolch/service/api/DefaultServiceHandler.java b/li.strolch.agent/src/main/java/li/strolch/service/api/DefaultServiceHandler.java
index 5ca04920f..c4861278d 100644
--- a/li.strolch.agent/src/main/java/li/strolch/service/api/DefaultServiceHandler.java
+++ b/li.strolch.agent/src/main/java/li/strolch/service/api/DefaultServiceHandler.java
@@ -142,6 +142,22 @@ public class DefaultServiceHandler extends StrolchComponent implements ServiceHa
 		}
 	}
 
+	private String getRealmName(ServiceArgument arg, Certificate certificate) {
+		if (arg == null) {
+			return isNotEmpty(certificate.getRealm()) ?
+					certificate.getRealm() :
+					getContainer().getRealmNames().iterator().next();
+		}
+
+		if (isNotEmpty(arg.realm))
+			return arg.realm;
+
+		if (isNotEmpty(certificate.getRealm()))
+			return certificate.getRealm();
+
+		return getContainer().getRealmNames().iterator().next();
+	}
+
 	private void logResult(Service<?, ?> service, ServiceArgument arg, long start, Certificate certificate,
 			ServiceResult serviceResult) {
 
@@ -150,11 +166,8 @@ public class DefaultServiceHandler extends StrolchComponent implements ServiceHa
 		String msg = "User {0}: Service {1} took {2}"; //$NON-NLS-1$
 		String username = certificate.getUsername();
 		String svcName = service.getClass().getName();
-		String realmName = isNotEmpty(arg.realm) ?
-				arg.realm :
-				isNotEmpty(certificate.getRealm()) ?
-						certificate.getRealm() :
-						getContainer().getRealmNames().iterator().next();
+
+		String realmName = getRealmName(arg, certificate);
 
 		msg = MessageFormat.format(msg, username, svcName, formatNanoDuration(end - start));
 
diff --git a/li.strolch.service/src/test/java/li/strolch/service/TxTest.java b/li.strolch.service/src/test/java/li/strolch/service/TxTest.java
index e4cd8d6e4..a5f245f1b 100644
--- a/li.strolch.service/src/test/java/li/strolch/service/TxTest.java
+++ b/li.strolch.service/src/test/java/li/strolch/service/TxTest.java
@@ -204,7 +204,7 @@ public class TxTest extends AbstractRealmServiceTest<ServiceArgument, ServiceRes
 			Resource resource = ModelGenerator.createResource(id, id, id);
 
 			boolean txFailed = false;
-			try (StrolchTransaction tx = openTx(arg.realm)) {
+			try (StrolchTransaction tx = openTx(arg.realm, true)) {
 				AddResourceCommand addResCmd = new AddResourceCommand(getContainer(), tx);
 				addResCmd.setResource(resource);
 				tx.addCommand(addResCmd);
diff --git a/li.strolch.service/src/test/java/li/strolch/service/test/ServiceTest.java b/li.strolch.service/src/test/java/li/strolch/service/test/ServiceTest.java
index 5c2bd2d64..657714c28 100644
--- a/li.strolch.service/src/test/java/li/strolch/service/test/ServiceTest.java
+++ b/li.strolch.service/src/test/java/li/strolch/service/test/ServiceTest.java
@@ -44,13 +44,6 @@ public class ServiceTest extends AbstractServiceTest {
 	@Rule
 	public ExpectedException thrown = ExpectedException.none();
 
-	@Test
-	public void shouldFailNoCertificate() {
-		TestService testService = new TestService();
-		ServiceResult svcResult = getServiceHandler().doService(null, testService);
-		assertThat(svcResult.getThrowable(), instanceOf(PrivilegeException.class));
-	}
-
 	@Test
 	public void shouldFailInvalidCertificate1() {
 		this.thrown.expect(PrivilegeException.class);