From 2215bfe83d00ed2cf562177627103dd80f0f0509 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20K=C3=B6rber?= Date: Sun, 27 Oct 2019 14:47:23 +0100 Subject: [PATCH] Expect JSON results to be always present This saves an unnecessary and annoying null dereference check. --- .../java/org/shredzone/acme4j/Account.java | 17 +++----------- .../org/shredzone/acme4j/AccountBuilder.java | 6 +---- .../shredzone/acme4j/AcmeJsonResource.java | 5 +---- .../org/shredzone/acme4j/Authorization.java | 7 +----- .../main/java/org/shredzone/acme4j/Login.java | 4 +--- .../main/java/org/shredzone/acme4j/Order.java | 7 +----- .../org/shredzone/acme4j/OrderBuilder.java | 6 +---- .../shredzone/acme4j/challenge/Challenge.java | 6 +---- .../acme4j/connector/Connection.java | 6 +++-- .../acme4j/connector/DefaultConnection.java | 22 +++++++------------ .../acme4j/connector/ResourceIterator.java | 6 +---- 11 files changed, 23 insertions(+), 69 deletions(-) diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/Account.java b/acme4j-client/src/main/java/org/shredzone/acme4j/Account.java index 60c20fb7..3f4ae73d 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/Account.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/Account.java @@ -203,10 +203,7 @@ public class Account extends AcmeJsonResource { } Authorization auth = getLogin().bindAuthorization(authLocation); - JSON json = conn.readJsonResponse(); - if (json != null) { - auth.setJSON(json); - } + auth.setJSON(conn.readJsonResponse()); return auth; } } @@ -258,11 +255,7 @@ public class Account extends AcmeJsonResource { claims.put(KEY_STATUS, "deactivated"); conn.sendSignedRequest(getLocation(), claims, getLogin()); - - JSON json = conn.readJsonResponse(); - if (json != null) { - setJSON(json); - } + setJSON(conn.readJsonResponse()); } } @@ -348,11 +341,7 @@ public class Account extends AcmeJsonResource { } conn.sendSignedRequest(getLocation(), claims, getLogin()); - - JSON json = conn.readJsonResponse(); - if (json != null) { - setJSON(json); - } + setJSON(conn.readJsonResponse()); } } } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/AccountBuilder.java b/acme4j-client/src/main/java/org/shredzone/acme4j/AccountBuilder.java index 243ba595..8fe44de7 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/AccountBuilder.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/AccountBuilder.java @@ -30,7 +30,6 @@ import org.shredzone.acme4j.connector.Resource; import org.shredzone.acme4j.exception.AcmeException; import org.shredzone.acme4j.exception.AcmeProtocolException; import org.shredzone.acme4j.toolbox.AcmeUtils; -import org.shredzone.acme4j.toolbox.JSON; import org.shredzone.acme4j.toolbox.JSONBuilder; import org.shredzone.acme4j.toolbox.JoseUtils; import org.slf4j.Logger; @@ -219,10 +218,7 @@ public class AccountBuilder { } Login login = new Login(location, keyPair, session); - JSON json = conn.readJsonResponse(); - if (json != null) { - login.getAccount().setJSON(json); - } + login.getAccount().setJSON(conn.readJsonResponse()); return login; } } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeJsonResource.java b/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeJsonResource.java index 1d8b6a5f..99d2f849 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeJsonResource.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeJsonResource.java @@ -117,10 +117,7 @@ public abstract class AcmeJsonResource extends AcmeResource { LOG.debug("update {}", resourceType); try (Connection conn = getSession().connect()) { conn.sendSignedPostAsGetRequest(getLocation(), getLogin()); - JSON json = conn.readJsonResponse(); - if (json != null) { - setJSON(json); - } + setJSON(conn.readJsonResponse()); conn.handleRetryAfter(resourceType + " is not completed yet"); } } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/Authorization.java b/acme4j-client/src/main/java/org/shredzone/acme4j/Authorization.java index 621d87fa..90aa45e4 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/Authorization.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/Authorization.java @@ -28,7 +28,6 @@ import org.shredzone.acme4j.connector.Connection; import org.shredzone.acme4j.exception.AcmeException; import org.shredzone.acme4j.exception.AcmeProtocolException; import org.shredzone.acme4j.toolbox.AcmeUtils; -import org.shredzone.acme4j.toolbox.JSON; import org.shredzone.acme4j.toolbox.JSON.Value; import org.shredzone.acme4j.toolbox.JSONBuilder; import org.slf4j.Logger; @@ -156,11 +155,7 @@ public class Authorization extends AcmeJsonResource { claims.put("status", "deactivated"); conn.sendSignedRequest(getLocation(), claims, getLogin()); - - JSON json = conn.readJsonResponse(); - if (json != null) { - setJSON(json); - } + setJSON(conn.readJsonResponse()); } } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/Login.java b/acme4j-client/src/main/java/org/shredzone/acme4j/Login.java index c1ad5093..cbb850ee 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/Login.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/Login.java @@ -135,9 +135,7 @@ public class Login { public Challenge bindChallenge(URL location) throws AcmeException { Connection connect = session.connect(); connect.sendSignedPostAsGetRequest(location, this); - JSON data = connect.readJsonResponse(); - Objects.requireNonNull(data, "data"); - return createChallenge(data); + return createChallenge(connect.readJsonResponse()); } /** diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/Order.java b/acme4j-client/src/main/java/org/shredzone/acme4j/Order.java index 66fbd20e..395c41e4 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/Order.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/Order.java @@ -26,7 +26,6 @@ import javax.annotation.ParametersAreNonnullByDefault; import org.shredzone.acme4j.connector.Connection; import org.shredzone.acme4j.exception.AcmeException; -import org.shredzone.acme4j.toolbox.JSON; import org.shredzone.acme4j.toolbox.JSON.Value; import org.shredzone.acme4j.toolbox.JSONBuilder; import org.slf4j.Logger; @@ -266,11 +265,7 @@ public class Order extends AcmeJsonResource { claims.put("status", "canceled"); conn.sendSignedRequest(getLocation(), claims, getLogin()); - - JSON json = conn.readJsonResponse(); - if (json != null) { - setJSON(json); - } + setJSON(conn.readJsonResponse()); } } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/OrderBuilder.java b/acme4j-client/src/main/java/org/shredzone/acme4j/OrderBuilder.java index 905d6688..43590f97 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/OrderBuilder.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/OrderBuilder.java @@ -29,7 +29,6 @@ import org.shredzone.acme4j.connector.Connection; import org.shredzone.acme4j.connector.Resource; import org.shredzone.acme4j.exception.AcmeException; import org.shredzone.acme4j.exception.AcmeProtocolException; -import org.shredzone.acme4j.toolbox.JSON; import org.shredzone.acme4j.toolbox.JSONBuilder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -322,10 +321,7 @@ public class OrderBuilder { } Order order = new Order(login, orderLocation); - JSON json = conn.readJsonResponse(); - if (json != null) { - order.setJSON(json); - } + order.setJSON(conn.readJsonResponse()); return order; } } 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 55d1fbda..7029789f 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 @@ -153,11 +153,7 @@ public class Challenge extends AcmeJsonResource { prepareResponse(claims); conn.sendSignedRequest(getLocation(), claims, getLogin()); - - JSON json = conn.readJsonResponse(); - if (json != null) { - setJSON(json); - } + setJSON(conn.readJsonResponse()); } } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/Connection.java b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/Connection.java index 2d5cb9d7..d905f0f2 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/Connection.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/Connection.java @@ -26,6 +26,7 @@ import javax.annotation.ParametersAreNonnullByDefault; import org.shredzone.acme4j.Login; import org.shredzone.acme4j.Session; import org.shredzone.acme4j.exception.AcmeException; +import org.shredzone.acme4j.exception.AcmeProtocolException; import org.shredzone.acme4j.exception.AcmeRetryAfterException; import org.shredzone.acme4j.toolbox.JSON; import org.shredzone.acme4j.toolbox.JSONBuilder; @@ -130,9 +131,10 @@ public interface Connection extends AutoCloseable { /** * Reads a server response as JSON data. * - * @return The JSON response, or {@code null} if the server did not provide any data. + * @return The JSON response. + * @throws AcmeProtocolException + * if the JSON response was empty. */ - @CheckForNull JSON readJsonResponse() throws AcmeException; /** 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 bd94c7b6..286ee89d 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 @@ -161,12 +161,11 @@ public class DefaultConnection implements Connection { } @Override - @CheckForNull public JSON readJsonResponse() throws AcmeException { assertConnectionIsOpen(); if (conn.getContentLength() == 0) { - return null; + throw new AcmeProtocolException("Empty response"); } String contentType = AcmeUtils.getContentType(conn.getHeaderField(CONTENT_TYPE_HEADER)); @@ -174,20 +173,19 @@ public class DefaultConnection implements Connection { throw new AcmeProtocolException("Unexpected content type: " + contentType); } - JSON result = null; - try { InputStream in = conn.getResponseCode() < 400 ? conn.getInputStream() : conn.getErrorStream(); - if (in != null) { - result = JSON.parse(in); - LOG.debug("Result JSON: {}", result.toString()); + if (in == null) { + throw new AcmeProtocolException("JSON response is empty"); } + + JSON result = JSON.parse(in); + LOG.debug("Result JSON: {}", result.toString()); + return result; } catch (IOException ex) { throw new AcmeNetworkException(ex); } - - return result; } @Override @@ -461,11 +459,7 @@ public class DefaultConnection implements Connection { throw new AcmeException("HTTP " + conn.getResponseCode() + ": " + conn.getResponseMessage()); } - JSON problemJson = readJsonResponse(); - if (problemJson == null) { - throw new AcmeProtocolException("Empty problem response"); - } - Problem problem = new Problem(problemJson, conn.getURL()); + Problem problem = new Problem(readJsonResponse(), conn.getURL()); String error = AcmeUtils.stripErrorPrefix(problem.getType().toString()); diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/ResourceIterator.java b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/ResourceIterator.java index 568d02a1..0463617f 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/ResourceIterator.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/ResourceIterator.java @@ -146,11 +146,7 @@ public class ResourceIterator implements Iterator { Session session = login.getSession(); try (Connection conn = session.connect()) { conn.sendSignedPostAsGetRequest(nextUrl, login); - - JSON json = conn.readJsonResponse(); - if (json != null) { - fillUrlList(json); - } + fillUrlList(conn.readJsonResponse()); nextUrl = conn.getLinks("next").stream().findFirst().orElse(null); }