Simplified challenge creation

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<String, Class<? extends Challenge>> 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 extends Challenge> T createChallenge(String type) {
-        Class<? extends Challenge> 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<? extends Challenge> 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<String> getRegisteredChallengeTypes() {
-        return Collections.unmodifiableCollection(challenges.keySet());
-    }
-
-    /**
-     * Registers all standard challenges as specified in the ACME specifications.
-     * <p>
-     * 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.
      *

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 extends Challenge> T createChallenge(String type);
 

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
+        }
     }
 
 }