Merge pull request #910 from jmartisk/main-issue-903

Unify client errors for missing URL, suggest a different fix if runni…
smallrye · Jul 13, 2021 · 2a55e37 · 2a55e37
2 parents 7c390fd + f31e649
commit 2a55e37
Show file tree

Hide file tree

Showing 8 changed files with 96 additions and 17 deletions.
diff --git a/...api/src/main/java/io/smallrye/graphql/client/dynamic/api/DynamicGraphQLClientBuilder.java b/...api/src/main/java/io/smallrye/graphql/client/dynamic/api/DynamicGraphQLClientBuilder.java
@@ -8,6 +8,8 @@ public interface DynamicGraphQLClientBuilder {
 
     DynamicGraphQLClientBuilder url(String url);
 
+    DynamicGraphQLClientBuilder configKey(String configKey);
+
     DynamicGraphQLClientBuilder header(String key, String value);
 
     DynamicGraphQLClient build();

diff --git a/...ain/java/io/smallrye/graphql/client/typesafe/jaxrs/JaxRsTypesafeGraphQLClientBuilder.java b/...ain/java/io/smallrye/graphql/client/typesafe/jaxrs/JaxRsTypesafeGraphQLClientBuilder.java
@@ -10,6 +10,7 @@
 import javax.ws.rs.client.ClientBuilder;
 import javax.ws.rs.client.WebTarget;
 
+import io.smallrye.graphql.client.ErrorMessageProvider;
 import io.smallrye.graphql.client.GraphQLClientConfiguration;
 import io.smallrye.graphql.client.GraphQLClientsConfiguration;
 import io.smallrye.graphql.client.typesafe.api.GraphQLClientApi;
@@ -77,6 +78,9 @@ public <T> T build(Class<T> apiClass) {
         if (persistentConfig != null) {
             applyConfig(persistentConfig);
         }
+        if (endpoint == null) {
+            throw ErrorMessageProvider.get().urlMissingErrorForNamedClient(configKey);
+        }
 
         WebTarget webTarget = client().target(endpoint);
         JaxRsTypesafeGraphQLClientProxy graphQlClient = new JaxRsTypesafeGraphQLClientProxy(webTarget, persistentConfig);

diff --git a/.../main/java/io/smallrye/graphql/client/dynamic/vertx/VertxDynamicGraphQLClientBuilder.java b/.../main/java/io/smallrye/graphql/client/dynamic/vertx/VertxDynamicGraphQLClientBuilder.java
@@ -1,5 +1,11 @@
 package io.smallrye.graphql.client.dynamic.vertx;
 
+import javax.enterprise.inject.spi.CDI;
+
+import io.smallrye.graphql.client.ErrorMessageProvider;
+import io.smallrye.graphql.client.GraphQLClientConfiguration;
+import io.smallrye.graphql.client.GraphQLClientsConfiguration;
+import io.smallrye.graphql.client.SmallRyeGraphQLClientMessages;
 import io.smallrye.graphql.client.dynamic.api.DynamicGraphQLClient;
 import io.smallrye.graphql.client.dynamic.api.DynamicGraphQLClientBuilder;
 import io.vertx.core.MultiMap;
@@ -14,6 +20,7 @@ public class VertxDynamicGraphQLClientBuilder implements DynamicGraphQLClientBui
 
     private Vertx vertx;
     private String url;
+    private String configKey;
     private final MultiMap headersMap;
     private WebClientOptions options;
 
@@ -43,13 +50,51 @@ public DynamicGraphQLClientBuilder url(String url) {
         return this;
     }
 
+    @Override
+    public DynamicGraphQLClientBuilder configKey(String configKey) {
+        this.configKey = configKey;
+        return this;
+    }
+
     @Override
     public DynamicGraphQLClient build() {
+        if (configKey != null) {
+            GraphQLClientConfiguration persistentConfig;
+            try {
+                persistentConfig = CDI.current()
+                        .select(GraphQLClientsConfiguration.class).get()
+                        .getClients().get(configKey);
+            } catch (IllegalStateException ex) {
+                persistentConfig = null;
+            }
+            if (persistentConfig != null) {
+                applyConfig(persistentConfig);
+            }
+        }
         if (url == null) {
-            throw new IllegalArgumentException("URL is required");
+            if (configKey == null) {
+                throw SmallRyeGraphQLClientMessages.msg.urlNotConfiguredForProgrammaticClient();
+            } else {
+                throw ErrorMessageProvider.get().urlMissingErrorForNamedClient(configKey);
+            }
         }
         Vertx toUseVertx = vertx != null ? vertx : Vertx.vertx();
         return new VertxDynamicGraphQLClient(toUseVertx, url, headersMap, options);
     }
 
+    /**
+     * Applies values from known global configuration. This does NOT override values passed to this
+     * builder by method calls.
+     */
+    private void applyConfig(GraphQLClientConfiguration configuration) {
+        if (this.url == null && configuration.getUrl() != null) {
+            this.url = configuration.getUrl();
+        }
+        configuration.getHeaders().forEach((k, v) -> {
+            if (!this.headersMap.contains(k)) {
+                this.headersMap.set(k, v);
+            }
+        });
+    }
+
 }
diff --git a/.../implementation/src/main/java/io/smallrye/graphql/client/DefaultErrorMessageProvider.java b/.../implementation/src/main/java/io/smallrye/graphql/client/DefaultErrorMessageProvider.java
@@ -0,0 +1,10 @@
+package io.smallrye.graphql.client;
+
+public class DefaultErrorMessageProvider implements ErrorMessageProvider {
+
+    @Override
+    public RuntimeException urlMissingErrorForNamedClient(String name) {
+        return SmallRyeGraphQLClientMessages.msg.urlNotConfiguredForNamedClient(name);
+    }
+
+}
diff --git a/client/implementation/src/main/java/io/smallrye/graphql/client/ErrorMessageProvider.java b/client/implementation/src/main/java/io/smallrye/graphql/client/ErrorMessageProvider.java
@@ -0,0 +1,26 @@
+package io.smallrye.graphql.client;
+
+import java.util.ServiceLoader;
+
+/**
+ * Some error messages might differ depending on what appserver/runtime we are running inside.
+ * SmallRye provides a default error message provider, but some runtimes may choose to override it.
+ */
+public interface ErrorMessageProvider {
+
+    RuntimeException urlMissingErrorForNamedClient(String name);
+
+    static ErrorMessageProvider get() {
+        ServiceLoader<ErrorMessageProvider> providers = ServiceLoader.load(ErrorMessageProvider.class);
+        ErrorMessageProvider chosenProvider = null;
+        for (ErrorMessageProvider provider : providers) {
+            // pick the a provided non-default one, if it exists
+            // otherwise use the default provider
+            if (chosenProvider == null || !provider.getClass().equals(DefaultErrorMessageProvider.class)) {
+                chosenProvider = provider;
+            }
+        }
+        return chosenProvider;
+    }
+
+}
diff --git a/...mplementation/src/main/java/io/smallrye/graphql/client/SmallRyeGraphQLClientMessages.java b/...mplementation/src/main/java/io/smallrye/graphql/client/SmallRyeGraphQLClientMessages.java
@@ -18,7 +18,11 @@ public interface SmallRyeGraphQLClientMessages {
     @Message(id = 35000, value = "Cannot parse response: %s")
     DynamicClientException cannotParseResponse(String response);
 
-    @Message(id = 35001, value = "URL not configured for client named '%s'. Please define the property smallrye.graphql.client.CLIENT_NAME/url")
+    @Message(id = 35001, value = "URL not configured for client. Please define the property %s/mp-graphql/url or pass it to " +
+            "your client builder dynamically")
     NoSuchElementException urlNotConfiguredForNamedClient(String name);
 
+    @Message(id = 35002, value = "URL not configured for client. Please pass a url using your client builder.")
+    NoSuchElementException urlNotConfiguredForProgrammaticClient();
+
 }
diff --git a/...lementation/src/main/java/io/smallrye/graphql/client/dynamic/cdi/NamedDynamicClients.java b/...lementation/src/main/java/io/smallrye/graphql/client/dynamic/cdi/NamedDynamicClients.java
@@ -13,9 +13,7 @@
 import javax.inject.Inject;
 
 import io.smallrye.graphql.client.GraphQLClient;
-import io.smallrye.graphql.client.GraphQLClientConfiguration;
 import io.smallrye.graphql.client.GraphQLClientsConfiguration;
-import io.smallrye.graphql.client.SmallRyeGraphQLClientMessages;
 import io.smallrye.graphql.client.dynamic.api.DynamicGraphQLClient;
 import io.smallrye.graphql.client.dynamic.api.DynamicGraphQLClientBuilder;
 
@@ -40,19 +38,8 @@ void initialize() {
     DynamicGraphQLClient getClient(InjectionPoint ip) {
         GraphQLClient annotation = ip.getAnnotated().getAnnotation(GraphQLClient.class);
         String clientName = annotation != null ? annotation.value() : DEFAULT_CLIENT_NAME;
-        return createdClients.computeIfAbsent(clientName, name -> {
-            GraphQLClientConfiguration config = globalConfig.getClients().get(name);
-            if (config == null || config.getUrl() == null) {
-                throw SmallRyeGraphQLClientMessages.msg.urlNotConfiguredForNamedClient(clientName);
-            }
-
-            DynamicGraphQLClientBuilder builder = DynamicGraphQLClientBuilder.newBuilder();
-            builder = builder.url(config.getUrl());
-            for (Map.Entry<String, String> headers : config.getHeaders().entrySet()) {
-                builder = builder.header(headers.getKey(), headers.getValue());
-            }
-            return builder.build();
-        });
+        return createdClients.computeIfAbsent(clientName,
+                name -> DynamicGraphQLClientBuilder.newBuilder().configKey(name).build());
     }
 
     @PreDestroy

diff --git a/...tion/src/main/resources/META-INF/services/io.smallrye.graphql.client.ErrorMessageProvider b/...tion/src/main/resources/META-INF/services/io.smallrye.graphql.client.ErrorMessageProvider
@@ -0,0 +1 @@
+io.smallrye.graphql.client.DefaultErrorMessageProvider
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		io.smallrye.graphql.client.DefaultErrorMessageProvider