simplify error reporting

eclipse-edc · Nov 14, 2024 · c9977de · c9977de
1 parent 49c77af
commit c9977de
Show file tree

Hide file tree

Showing 8 changed files with 15 additions and 24 deletions.
diff --git a/core/common/boot/src/main/java/org/eclipse/edc/boot/system/DependencyGraph.java b/core/common/boot/src/main/java/org/eclipse/edc/boot/system/DependencyGraph.java
@@ -26,6 +26,7 @@
 import org.eclipse.edc.runtime.metamodel.annotation.Requires;
 import org.eclipse.edc.spi.system.ServiceExtension;
 import org.eclipse.edc.spi.system.ServiceExtensionContext;
+import org.jetbrains.annotations.Nullable;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -131,7 +132,7 @@ public static DependencyGraph of(ServiceExtensionContext context, List<ServiceEx
                                 .onFailure(f -> {
                                     if (injectionPoint.isRequired()) {
                                         unsatisfiedInjectionPoints.computeIfAbsent(injectionPoint.getTargetInstance().getClass(), s -> new ArrayList<>())
-                                                .add(new InjectionFailure(injectionPoint, injectionPoint.getTypeString() + " " + f.getFailureDetail()));
+                                                .add(new InjectionFailure(injectionPoint.getTargetInstance(), injectionPoint, f.getFailureDetail()));
                                     }
                                 });
 
@@ -225,13 +226,12 @@ public List<String> getProblems() {
                     var dependent = entry.getKey();
                     var dependencies = entry.getValue();
                     var missingDependencyList = dependencies.stream().map(injectionFailure -> "  --> " + injectionFailure.failureDetail()).toList();
-                    return "-- %s is missing\n%s".formatted(dependent, String.join("\n", missingDependencyList));
+                    return "## %s is missing\n%s".formatted(dependent, String.join("\n", missingDependencyList));
                 });
 
         return messages.toList();
     }
 
-
     private static Stream<Class<?>> getRequiredFeatures(Class<?> clazz) {
         var requiresAnnotation = clazz.getAnnotation(Requires.class);
         if (requiresAnnotation != null) {
@@ -256,10 +256,11 @@ private static Set<Class<?>> getProvidedFeatures(ServiceExtension ext) {
         return allProvides;
     }
 
-    private record InjectionFailure(InjectionPoint<ServiceExtension> injectionPoint, String failureDetail) {
+    private record InjectionFailure(ServiceExtension dependent, InjectionPoint<ServiceExtension> dependency,
+                                    @Nullable String failureDetail) {
         @Override
         public String toString() {
-            return "%s %s".formatted(injectionPoint.getTypeString(), failureDetail);
+            return "%s %s".formatted(dependency.getTypeString(), failureDetail);
         }
     }
 }
diff --git a/...boot/src/main/java/org/eclipse/edc/boot/system/injection/ConfigurationInjectionPoint.java b/...boot/src/main/java/org/eclipse/edc/boot/system/injection/ConfigurationInjectionPoint.java
@@ -159,7 +159,7 @@ public Result<List<InjectionContainer<T>>> getProviders(Map<Class<?>, List<Injec
                 .filter(Result::failed)
                 .map(AbstractResult::getFailureDetail)
                 .toList();
-        return violators.isEmpty() ? Result.success(List.of()) : Result.failure("'%s' (%s), through nested settings %s".formatted(configurationObject.getName(), configurationObject.getType().getSimpleName(), violators));
+        return violators.isEmpty() ? Result.success(List.of()) : Result.failure("%s, through nested settings %s".formatted(toString(), violators));
     }
 
     @Override
@@ -169,8 +169,8 @@ public String getTypeString() {
 
     @Override
     public String toString() {
-        return "Configuration '%s' of type '%s' in %s"
-                .formatted(configurationObject.getName(), configurationObject.getType(), targetInstance.getClass());
+        return "Configuration object \"%s\" of type [%s]"
+                .formatted(configurationObject.getName(), configurationObject.getType());
     }
 
     private Predicate<Constructor<?>> constructorFilter(List<FieldValue> args) {

diff --git a/...ommon/boot/src/main/java/org/eclipse/edc/boot/system/injection/ServiceInjectionPoint.java b/...ommon/boot/src/main/java/org/eclipse/edc/boot/system/injection/ServiceInjectionPoint.java
@@ -106,7 +106,7 @@ public Result<List<InjectionContainer<T>>> getProviders(Map<Class<?>, List<Injec
             return Result.success(List.of());
         } else {
             // attempt to interpret the feature name as class name and see if the context has that service
-            return Result.failure("'" + injectedField.getName() + "'" + " of type " + serviceClass);
+            return Result.failure(toString());
         }
 
     }
@@ -118,6 +118,6 @@ public String getTypeString() {
 
     @Override
     public String toString() {
-        return format("Field \"%s\" of type [%s] required by %s", injectedField.getName(), getType(), instance.getClass().getName());
+        return format("Field \"%s\" of type [%s]", injectedField.getName(), getType());
     }
 }
diff --git a/.../common/boot/src/main/java/org/eclipse/edc/boot/system/injection/ValueInjectionPoint.java b/.../common/boot/src/main/java/org/eclipse/edc/boot/system/injection/ValueInjectionPoint.java
@@ -177,7 +177,7 @@ public Result<List<InjectionContainer<T>>> getProviders(Map<Class<?>, List<Injec
         // no default value, the required value may be found in the config
         return context.getConfig().hasKey(annotationValue.key())
                 ? Result.success(emptyProviderlist)
-                : Result.failure("%s (property \"%s\")".formatted(targetField.getName(), annotationValue.key()));
+                : Result.failure(toString());
     }
 
     @Override
@@ -187,7 +187,7 @@ public String getTypeString() {
 
     @Override
     public String toString() {
-        return "Configuration value \"%s\" of type %s (config key '%s') in %s".formatted(targetField.getName(), getType(), annotationValue.key(), declaringClass);
+        return "Configuration value \"%s\" of type [%s] (property '%s')".formatted(targetField.getName(), getType(), annotationValue.key());
     }
 
     private Object parseEntry(String string, Class<?> valueType) {

diff --git a/core/common/boot/src/test/java/org/eclipse/edc/boot/system/ExtensionLoaderTest.java b/core/common/boot/src/test/java/org/eclipse/edc/boot/system/ExtensionLoaderTest.java
@@ -231,8 +231,7 @@ void buildDependencyGraph_dependencyNotSatisfied() {
 
         var graph = loader.buildDependencyGraph(context);
         assertThat(graph.isValid()).isFalse();
-        assertThat(graph.getProblems())
-                .contains("class org.eclipse.edc.boot.system.ExtensionLoaderTest$DependingExtension --> Service someService of type class org.eclipse.edc.boot.system.ExtensionLoaderTest$SomeObject");
+        assertThat(graph.getProblems()).hasSize(1);
         verify(serviceLocator).loadImplementors(eq(ServiceExtension.class), anyBoolean());
     }
 

diff --git a/.../src/test/java/org/eclipse/edc/boot/system/injection/ConfigurationInjectionPointTest.java b/.../src/test/java/org/eclipse/edc/boot/system/injection/ConfigurationInjectionPointTest.java
@@ -115,6 +115,6 @@ void getProviders_hasViolations() {
 
         var result = injectionPoint.getProviders(Map.of(), context);
         assertThat(result.succeeded()).isFalse();
-        assertThat(result.getFailureDetail()).isEqualTo("configurationObject (ConfigurationObject) --> [requiredVal (property \"foo.bar.baz\")]");
+        assertThat(result.getFailureDetail()).isEqualTo("Configuration object \"configurationObject\" of type [class org.eclipse.edc.boot.system.testextensions.ConfigurationObject], through nested settings [Configuration value \"requiredVal\" of type [class java.lang.String] (property 'foo.bar.baz')]");
     }
 }
diff --git a/...on/connector-core/src/main/java/org/eclipse/edc/connector/core/CoreServicesExtension.java b/...on/connector-core/src/main/java/org/eclipse/edc/connector/core/CoreServicesExtension.java
@@ -69,9 +69,6 @@ public class CoreServicesExtension implements ServiceExtension {
     @Setting(description = "Connector hostname, which e.g. is used in referer urls", defaultValue = DEFAULT_EDC_HOSTNAME, key = EDC_HOSTNAME, warnOnMissingConfig = true)
     public static String hostname;
 
-    @Setting(key = "bar.baz")
-    public static Long barBaz;
-
     @Inject
     private EventExecutorServiceContainer eventExecutorServiceContainer;
 

diff --git a/...clipse/edc/iam/identitytrust/sts/remote/client/StsRemoteClientConfigurationExtension.java b/...clipse/edc/iam/identitytrust/sts/remote/client/StsRemoteClientConfigurationExtension.java
@@ -48,17 +48,11 @@ public StsRemoteClientConfiguration clientConfiguration(ServiceExtensionContext
         return new StsRemoteClientConfiguration(clientConfig.tokenUrl(), clientConfig.clientId(), clientConfig.clientSecretAlias());
     }
 
-    @Inject
-    private FooService fooService;
-
     @Settings
     private record StsClientConfig(
             @Setting(key = "edc.iam.sts.oauth.token.url", description = "STS OAuth2 endpoint for requesting a token") String tokenUrl,
             @Setting(key = "edc.iam.sts.oauth.client.id", description = "STS OAuth2 client id") String clientId,
             @Setting(key = "edc.iam.sts.oauth.client.secret.alias", description = "Vault alias of STS OAuth2 client secret") String clientSecretAlias) {
     }
-
-    private class FooService {
-    }
 }