From c45c29226ed5e66e67b4b5f74916a387e88ae27d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20K=C3=B6rber?= Date: Wed, 16 Dec 2015 00:45:14 +0100 Subject: [PATCH] Simplified challenge creation --- .../provider/AbstractAcmeClientProvider.java | 70 ++-------------- .../acme4j/provider/AcmeClientProvider.java | 5 -- .../AbstractAcmeClientProviderTest.java | 79 ++++--------------- 3 files changed, 22 insertions(+), 132 deletions(-) diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AbstractAcmeClientProvider.java b/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AbstractAcmeClientProvider.java index b9f3a1f6..8fac0d51 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AbstractAcmeClientProvider.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AbstractAcmeClientProvider.java @@ -14,10 +14,6 @@ package org.shredzone.acme4j.provider; import java.net.URI; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; import org.shredzone.acme4j.AcmeClient; import org.shredzone.acme4j.challenge.Challenge; @@ -41,12 +37,6 @@ import org.shredzone.acme4j.impl.GenericAcmeClient; */ public abstract class AbstractAcmeClientProvider implements AcmeClientProvider { - private final Map> challenges = new HashMap<>(); - - public AbstractAcmeClientProvider() { - registerBaseChallenges(); - } - /** * Resolves the server URI and returns the matching directory URI. * @@ -70,16 +60,16 @@ public abstract class AbstractAcmeClientProvider implements AcmeClientProvider { @Override @SuppressWarnings("unchecked") public T createChallenge(String type) { - Class clazz = challenges.get(type); - if (clazz == null) { - return (T) new GenericChallenge(); + if (type == null || type.isEmpty()) { + throw new IllegalArgumentException("no type given"); } - try { - return (T) clazz.newInstance(); - } catch (InstantiationException | IllegalAccessException ex) { - throw new IllegalArgumentException("Could not create Challenge for type " - + type, ex); + switch (type) { + case DnsChallenge.TYPE: return (T) new DnsChallenge(); + case TlsSniChallenge.TYPE: return (T) new TlsSniChallenge(); + case ProofOfPossessionChallenge.TYPE: return (T) new ProofOfPossessionChallenge(); + case HttpChallenge.TYPE: return (T) new HttpChallenge(); + default: return (T) new GenericChallenge(); } } @@ -96,50 +86,6 @@ public abstract class AbstractAcmeClientProvider implements AcmeClientProvider { return new HttpConnector(); } - /** - * Registers an individual {@link Challenge}. If a challenge of that type is already - * registered, it will be replaced. - * - * @param type - * Challenge type string - * @param clazz - * Class implementing the {@link Challenge}. It must have a default - * constructor. - */ - protected void registerChallenge(String type, Class clazz) { - if (type == null) { - throw new NullPointerException("type must not be null"); - } - if (clazz == null) { - throw new NullPointerException("Challenge class must not be null"); - } - if (type.trim().isEmpty()) { - throw new IllegalArgumentException("type must not be empty"); - } - - challenges.put(type, clazz); - } - - /** - * Returns all registered challenge types. - */ - protected Collection getRegisteredChallengeTypes() { - return Collections.unmodifiableCollection(challenges.keySet()); - } - - /** - * Registers all standard challenges as specified in the ACME specifications. - *

- * Subclasses may override this method in order to add further challenges. It is - * invoked on construction time. - */ - protected void registerBaseChallenges() { - registerChallenge(DnsChallenge.TYPE, DnsChallenge.class); - registerChallenge(TlsSniChallenge.TYPE, TlsSniChallenge.class); - registerChallenge(ProofOfPossessionChallenge.TYPE, ProofOfPossessionChallenge.class); - registerChallenge(HttpChallenge.TYPE, HttpChallenge.class); - } - /** * Creates an {@link AcmeClient} for the given directory URI. * diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AcmeClientProvider.java b/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AcmeClientProvider.java index c0ebf4b8..b5f66d98 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AcmeClientProvider.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AcmeClientProvider.java @@ -18,7 +18,6 @@ import java.util.ServiceLoader; import org.shredzone.acme4j.AcmeClient; import org.shredzone.acme4j.challenge.Challenge; -import org.shredzone.acme4j.challenge.GenericChallenge; import org.shredzone.acme4j.connector.Connection; /** @@ -61,10 +60,6 @@ public interface AcmeClientProvider { * @throws ClassCastException * if the expected {@link Challenge} type does not match the given type * name. - * @throws IllegalArgumentException - * if the given type name cannot be resolved to any {@link Challenge} - * class. However, for unknown challenge types, a {@link GenericChallenge} - * instance should be returned. */ T createChallenge(String type); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/provider/AbstractAcmeClientProviderTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/provider/AbstractAcmeClientProviderTest.java index 37399ebe..56e5795a 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/provider/AbstractAcmeClientProviderTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/provider/AbstractAcmeClientProviderTest.java @@ -73,71 +73,6 @@ public class AbstractAcmeClientProviderTest { } } - /** - * Test that all base challenges are registered on initialization, and that additional - * challenges are properly registered. - */ - @Test - public void testRegisterChallenges() { - AbstractAcmeClientProvider provider = new AbstractAcmeClientProvider() { - @Override - protected void registerBaseChallenges() { - assertThat(getRegisteredChallengeTypes(), is(empty())); - super.registerBaseChallenges(); - } - - @Override - public boolean accepts(URI serverUri) { - throw new UnsupportedOperationException(); - } - - @Override - protected URI resolve(URI serverUri) { - throw new UnsupportedOperationException(); - } - }; - - assertThat(provider.getRegisteredChallengeTypes(), hasSize(4)); - assertThat(provider.getRegisteredChallengeTypes(), containsInAnyOrder( - DnsChallenge.TYPE, - HttpChallenge.TYPE, - ProofOfPossessionChallenge.TYPE, - TlsSniChallenge.TYPE - )); - - provider.registerChallenge("foo", GenericChallenge.class); - - assertThat(provider.getRegisteredChallengeTypes(), hasSize(5)); - assertThat(provider.getRegisteredChallengeTypes(), containsInAnyOrder( - DnsChallenge.TYPE, - HttpChallenge.TYPE, - ProofOfPossessionChallenge.TYPE, - TlsSniChallenge.TYPE, - "foo" - )); - - try { - provider.registerChallenge(null, GenericChallenge.class); - fail("accepts null type"); - } catch (NullPointerException ex) { - // expected - } - - try { - provider.registerChallenge("bar", null); - fail("accepts null class"); - } catch (NullPointerException ex) { - // expected - } - - try { - provider.registerChallenge("", GenericChallenge.class); - fail("accepts empty type"); - } catch (IllegalArgumentException ex) { - // expected - } - } - /** * Test that challenges are generated properly. */ @@ -177,6 +112,20 @@ public class AbstractAcmeClientProviderTest { Challenge c6 = provider.createChallenge("foobar-01"); assertThat(c6, not(nullValue())); assertThat(c6, instanceOf(GenericChallenge.class)); + + try { + provider.createChallenge(null); + fail("null was accepted"); + } catch (IllegalArgumentException ex) { + // expected + } + + try { + provider.createChallenge(""); + fail("empty string was accepted"); + } catch (IllegalArgumentException ex) { + // expected + } } }