From 232a243e92fa477a3bee82e60588aa5b62bd7632 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20K=C3=B6rber?= Date: Fri, 16 Dec 2016 01:19:25 +0100 Subject: [PATCH] Remove boilerplate code for parameter null checks --- .../org/shredzone/acme4j/AcmeResource.java | 12 +++------ .../org/shredzone/acme4j/Registration.java | 14 +++++----- .../java/org/shredzone/acme4j/Session.java | 18 ++++--------- .../shredzone/acme4j/challenge/Challenge.java | 9 +++---- .../acme4j/connector/DefaultConnection.java | 26 +++++-------------- .../acme4j/exception/AcmeServerException.java | 5 ++-- .../acme4j/provider/AbstractAcmeProvider.java | 9 +++---- .../org/shredzone/acme4j/util/AcmeUtils.java | 15 +++++++++++ .../shredzone/acme4j/util/ClaimBuilder.java | 9 ++----- .../provider/AbstractAcmeProviderTest.java | 2 +- .../shredzone/acme4j/util/AcmeUtilsTest.java | 17 +++++++++++- .../org/shredzone/acme4j/util/CSRBuilder.java | 4 +-- 12 files changed, 66 insertions(+), 74 deletions(-) diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeResource.java b/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeResource.java index 6bb0d971..c77fa320 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeResource.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeResource.java @@ -13,6 +13,8 @@ */ package org.shredzone.acme4j; +import static org.shredzone.acme4j.util.AcmeUtils.assertNotNull; + import java.io.Serializable; import java.net.URI; @@ -50,10 +52,7 @@ public abstract class AcmeResource implements Serializable { * Sets a new {@link Session}. */ protected void setSession(Session session) { - if (session == null) { - throw new NullPointerException("session must not be null"); - } - + assertNotNull(session, "session"); this.session = session; } @@ -61,10 +60,7 @@ public abstract class AcmeResource implements Serializable { * Sets the resource's location. */ protected void setLocation(URI location) { - if (location == null) { - throw new NullPointerException("location must not be null"); - } - + assertNotNull(location, "location"); this.location = location; } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/Registration.java b/acme4j-client/src/main/java/org/shredzone/acme4j/Registration.java index 89873ba9..4d8314a2 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/Registration.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/Registration.java @@ -36,6 +36,7 @@ import org.shredzone.acme4j.connector.ResourceIterator; import org.shredzone.acme4j.exception.AcmeException; import org.shredzone.acme4j.exception.AcmeProtocolException; import org.shredzone.acme4j.exception.AcmeRetryAfterException; +import org.shredzone.acme4j.util.AcmeUtils; import org.shredzone.acme4j.util.ClaimBuilder; import org.shredzone.acme4j.util.DomainUtils; import org.shredzone.acme4j.util.SignatureUtils; @@ -176,8 +177,9 @@ public class Registration extends AcmeResource { * @return {@link Authorization} object for this domain */ public Authorization authorizeDomain(String domain) throws AcmeException { - if (domain == null || domain.isEmpty()) { - throw new NullPointerException("domain must not be empty or null"); + AcmeUtils.assertNotNull(domain, "domain"); + if (domain.isEmpty()) { + throw new IllegalArgumentException("domain must not be empty"); } LOG.debug("authorizeDomain {}", domain); @@ -229,9 +231,7 @@ public class Registration extends AcmeResource { */ public Certificate requestCertificate(byte[] csr, Date notBefore, Date notAfter) throws AcmeException { - if (csr == null) { - throw new NullPointerException("csr must not be null"); - } + AcmeUtils.assertNotNull(csr, "csr"); LOG.debug("requestCertificate"); try (Connection conn = getSession().provider().connect()) { @@ -269,9 +269,7 @@ public class Registration extends AcmeResource { * new {@link KeyPair} to be used for identifying this account */ public void changeKey(KeyPair newKeyPair) throws AcmeException { - if (newKeyPair == null) { - throw new NullPointerException("newKeyPair must not be null"); - } + AcmeUtils.assertNotNull(newKeyPair, "newKeyPair"); if (Arrays.equals(getSession().getKeyPair().getPrivate().getEncoded(), newKeyPair.getPrivate().getEncoded())) { throw new IllegalArgumentException("newKeyPair must actually be a new key pair"); diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/Session.java b/acme4j-client/src/main/java/org/shredzone/acme4j/Session.java index 56b04f76..a76fd6e1 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/Session.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/Session.java @@ -30,6 +30,7 @@ import org.shredzone.acme4j.connector.Resource; import org.shredzone.acme4j.exception.AcmeException; import org.shredzone.acme4j.exception.AcmeProtocolException; import org.shredzone.acme4j.provider.AcmeProvider; +import org.shredzone.acme4j.util.AcmeUtils; /** * A session stores the ACME server URI and the account's key pair. It also tracks @@ -71,13 +72,8 @@ public class Session { * {@link KeyPair} of the ACME account */ public Session(URI serverUri, KeyPair keyPair) { - if (serverUri == null) { - throw new NullPointerException("serverUri must not be null"); - } - - if (keyPair == null) { - throw new NullPointerException("keypair must not be null"); - } + AcmeUtils.assertNotNull(serverUri, "serverUri"); + AcmeUtils.assertNotNull(keyPair, "keyPair"); this.serverUri = serverUri; this.keyPair = keyPair; @@ -170,9 +166,7 @@ public class Session { * @return {@link Challenge} instance */ public Challenge createChallenge(Map data) { - if (data == null) { - throw new NullPointerException("data must not be null"); - } + AcmeUtils.assertNotNull(data, "data"); String type = (String) data.get("type"); if (type == null || type.isEmpty()) { @@ -200,9 +194,7 @@ public class Session { * @return {@link URI}, or {@code null} if the server does not offer that resource */ public URI resourceUri(Resource resource) throws AcmeException { - if (resource == null) { - throw new NullPointerException("resource must not be null"); - } + AcmeUtils.assertNotNull(resource, "resource"); readDirectory(); return resourceMap.get(resource); } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/Challenge.java b/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/Challenge.java index 2672b203..dc477adc 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/Challenge.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/challenge/Challenge.java @@ -31,6 +31,7 @@ import org.shredzone.acme4j.connector.Connection; import org.shredzone.acme4j.exception.AcmeException; import org.shredzone.acme4j.exception.AcmeProtocolException; import org.shredzone.acme4j.exception.AcmeRetryAfterException; +import org.shredzone.acme4j.util.AcmeUtils; import org.shredzone.acme4j.util.ClaimBuilder; import org.shredzone.acme4j.util.TimestampParser; import org.slf4j.Logger; @@ -67,12 +68,8 @@ public class Challenge extends AcmeResource { */ @SuppressWarnings("unchecked") public static T bind(Session session, URI location) throws AcmeException { - if (session == null) { - throw new NullPointerException("session must not be null"); - } - if (location == null) { - throw new NullPointerException("location must not be null"); - } + AcmeUtils.assertNotNull(session, "session"); + AcmeUtils.assertNotNull(location, "location"); LOG.debug("bind"); try (Connection conn = session.provider().connect()) { diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java index 76c21c77..d7f5f3b8 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java @@ -49,6 +49,7 @@ import org.shredzone.acme4j.exception.AcmeProtocolException; import org.shredzone.acme4j.exception.AcmeRateLimitExceededException; import org.shredzone.acme4j.exception.AcmeServerException; import org.shredzone.acme4j.exception.AcmeUnauthorizedException; +import org.shredzone.acme4j.util.AcmeUtils; import org.shredzone.acme4j.util.ClaimBuilder; import org.shredzone.acme4j.util.SignatureUtils; import org.slf4j.Logger; @@ -71,21 +72,14 @@ public class DefaultConnection implements Connection { protected HttpURLConnection conn; public DefaultConnection(HttpConnector httpConnector) { - if (httpConnector == null) { - throw new NullPointerException("httpConnector must not be null"); - } - + AcmeUtils.assertNotNull(httpConnector, "httpConnector"); this.httpConnector = httpConnector; } @Override public void sendRequest(URI uri, Session session) throws AcmeException { - if (uri == null) { - throw new NullPointerException("uri must not be null"); - } - if (session == null) { - throw new NullPointerException("session must not be null"); - } + AcmeUtils.assertNotNull(uri, "uri"); + AcmeUtils.assertNotNull(session, "session"); assertConnectionIsClosed(); LOG.debug("GET {}", uri); @@ -107,15 +101,9 @@ public class DefaultConnection implements Connection { @Override public void sendSignedRequest(URI uri, ClaimBuilder claims, Session session) throws AcmeException { - if (uri == null) { - throw new NullPointerException("uri must not be null"); - } - if (claims == null) { - throw new NullPointerException("claims must not be null"); - } - if (session == null) { - throw new NullPointerException("session must not be null"); - } + AcmeUtils.assertNotNull(uri, "uri"); + AcmeUtils.assertNotNull(claims, "claims"); + AcmeUtils.assertNotNull(session, "session"); assertConnectionIsClosed(); try { diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/exception/AcmeServerException.java b/acme4j-client/src/main/java/org/shredzone/acme4j/exception/AcmeServerException.java index 7b40effe..b4cc5e41 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/exception/AcmeServerException.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/exception/AcmeServerException.java @@ -14,6 +14,7 @@ package org.shredzone.acme4j.exception; import org.shredzone.acme4j.connector.DefaultConnection; +import org.shredzone.acme4j.util.AcmeUtils; /** * An exception that is thrown when the ACME server returned an error. It contains @@ -35,9 +36,7 @@ public class AcmeServerException extends AcmeException { */ public AcmeServerException(String type, String detail) { super(detail); - if (type == null) { - throw new IllegalArgumentException("Error type must not be null"); - } + AcmeUtils.assertNotNull(type, "type"); this.type = type; } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AbstractAcmeProvider.java b/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AbstractAcmeProvider.java index e3bebd4c..a122c239 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AbstractAcmeProvider.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AbstractAcmeProvider.java @@ -27,6 +27,7 @@ import org.shredzone.acme4j.connector.Connection; import org.shredzone.acme4j.connector.DefaultConnection; import org.shredzone.acme4j.connector.HttpConnector; import org.shredzone.acme4j.exception.AcmeException; +import org.shredzone.acme4j.util.AcmeUtils; /** * Abstract implementation of {@link AcmeProvider}. It consists of a challenge @@ -58,11 +59,9 @@ public abstract class AbstractAcmeProvider implements AcmeProvider { @Override @SuppressWarnings("deprecation") // must still provide deprecated challenges public Challenge createChallenge(Session session, String type) { - if (session == null) { - throw new NullPointerException("session must not be null"); - } - - if (type == null || type.isEmpty()) { + AcmeUtils.assertNotNull(session, "session"); + AcmeUtils.assertNotNull(type, "type"); + if (type.isEmpty()) { throw new IllegalArgumentException("no type given"); } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/util/AcmeUtils.java b/acme4j-client/src/main/java/org/shredzone/acme4j/util/AcmeUtils.java index 0128602e..96ce9f67 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/util/AcmeUtils.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/util/AcmeUtils.java @@ -78,4 +78,19 @@ public final class AcmeUtils { return Base64Url.encode(data); } + /** + * Asserts that the given value is not {@code null}. Otherwise a + * {@link NullPointerException} is thrown. + * + * @param value + * Value to test + * @param name + * Name of the parameter + */ + public static void assertNotNull(Object value, String name) { + if (value == null) { + throw new NullPointerException(name + " must not be null"); + } + } + } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/util/ClaimBuilder.java b/acme4j-client/src/main/java/org/shredzone/acme4j/util/ClaimBuilder.java index 3cf669c0..8d63cf44 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/util/ClaimBuilder.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/util/ClaimBuilder.java @@ -56,10 +56,7 @@ public class ClaimBuilder { * @return {@code this} */ public ClaimBuilder put(String key, Object value) { - if (key == null) { - throw new NullPointerException("key must not be null"); - } - + AcmeUtils.assertNotNull(key, "key"); data.put(key, value); return this; } @@ -144,9 +141,7 @@ public class ClaimBuilder { * @return {@code this} */ public ClaimBuilder putKey(String key, PublicKey publickey) { - if (publickey == null) { - throw new NullPointerException("publickey must not be null"); - } + AcmeUtils.assertNotNull(publickey, "publickey"); try { final PublicJsonWebKey jwk = PublicJsonWebKey.Factory.newPublicJwk(publickey); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/provider/AbstractAcmeProviderTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/provider/AbstractAcmeProviderTest.java index dc589787..62026f1c 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/provider/AbstractAcmeProviderTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/provider/AbstractAcmeProviderTest.java @@ -165,7 +165,7 @@ public class AbstractAcmeProviderTest { try { provider.createChallenge(session, (String) null); fail("null was accepted"); - } catch (IllegalArgumentException ex) { + } catch (NullPointerException ex) { // expected } diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/util/AcmeUtilsTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/util/AcmeUtilsTest.java index 9a40d36e..c626616f 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/util/AcmeUtilsTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/util/AcmeUtilsTest.java @@ -14,7 +14,7 @@ package org.shredzone.acme4j.util; import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.*; import static org.shredzone.acme4j.util.AcmeUtils.*; import javax.xml.bind.DatatypeConverter; @@ -54,4 +54,19 @@ public class AcmeUtilsTest { assertThat(base64UrlEncode, is("w6uP8Tcg6K2QR905Rms8iXTlksL6OD1KOWBxTK7wxPI")); } + /** + * Test that {@code null} check works properly. + */ + @Test + public void testAssertNotNull() { + AcmeUtils.assertNotNull(new Object(), "foo"); + + try { + AcmeUtils.assertNotNull(null, "bar"); + fail("null was accepted"); + } catch (NullPointerException ex) { + // expected + } + } + } diff --git a/acme4j-utils/src/main/java/org/shredzone/acme4j/util/CSRBuilder.java b/acme4j-utils/src/main/java/org/shredzone/acme4j/util/CSRBuilder.java index fdc83232..97af4d95 100644 --- a/acme4j-utils/src/main/java/org/shredzone/acme4j/util/CSRBuilder.java +++ b/acme4j-utils/src/main/java/org/shredzone/acme4j/util/CSRBuilder.java @@ -145,12 +145,10 @@ public class CSRBuilder { * {@link KeyPair} to sign the CSR with */ public void sign(KeyPair keypair) throws IOException { + AcmeUtils.assertNotNull(keypair, "keypair"); if (namelist.isEmpty()) { throw new IllegalStateException("No domain was set"); } - if (keypair == null) { - throw new IllegalArgumentException("keypair must not be null"); - } try { GeneralName[] gns = new GeneralName[namelist.size()];