feat!: Remove hard dependency on MicroProfile Config from the core SDK #468
Conversation
Default values will now be used by default. You can supply your own by providing a CDI bean with a higher priority. Also, there is a new 2a-java-sdk-microprofile-config with the previous MicroProfile Config capabilities. If used, this will allow MicroProfile Config configurations of the properties. The reference implementations use this new module
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request does a great job of decoupling the core SDK from a hard dependency on MicroProfile Config by introducing the A2AConfigProvider abstraction. The new a2a-java-sdk-microprofile-config module is a clean way to provide this functionality optionally. The use of a2a-defaults.properties files for default values is a flexible approach.
I have a couple of suggestions to improve the implementation and documentation. One is a high-priority change to DefaultValuesConfigProvider to make it a CDI @Alternative, which will prevent ambiguous dependency issues and ensure the correct provider is selected automatically. The other is a minor correction to the documentation to more accurately describe how default properties are loaded.
Overall, this is a solid architectural improvement for the SDK.
| @ApplicationScoped | ||
| public class DefaultValuesConfigProvider implements A2AConfigProvider { |
There was a problem hiding this comment.
To ensure this provider acts as a fallback and to avoid ambiguous dependency issues when other A2AConfigProvider implementations are present, this class should be marked as a CDI @Alternative with a low @Priority. This allows other providers (like MicroProfileConfigProvider) with a higher priority to be automatically selected by CDI when available.
You'll also need to add the following imports:
import jakarta.annotation.Priority;
import jakarta.enterprise.inject.Alternative;| @ApplicationScoped | |
| public class DefaultValuesConfigProvider implements A2AConfigProvider { | |
| @ApplicationScoped | |
| @Alternative | |
| @Priority(1) | |
| public class DefaultValuesConfigProvider implements A2AConfigProvider { |
There was a problem hiding this comment.
This is not what we do elsewhere. I have added a test to ensure the MicroProfileConfigProvider gets used if on the classpath
|
/gemini review |
There was a problem hiding this comment.
Code Review
This is an excellent pull request that successfully decouples the core SDK from a hard dependency on MicroProfile Config. The introduction of the A2AConfigProvider interface, a default implementation that reads from a2a-defaults.properties files, and a separate a2a-java-sdk-microprofile-config integration module is a very clean and extensible design. This greatly improves the SDK's flexibility and makes it easier to integrate with various frameworks. The changes are consistent across the codebase, and the addition of documentation and tests for the new module is much appreciated.
I have a couple of minor suggestions to improve the documentation for custom providers and to make one of the new tests more robust. Please see the detailed comments.
| If you're using a different framework (Spring, Micronaut, etc.), you can implement your own `A2AConfigProvider`: | ||
|
|
||
| ```java | ||
| import io.a2a.server.config.A2AConfigProvider; | ||
| import io.a2a.server.config.DefaultValuesConfigProvider; | ||
| import jakarta.enterprise.context.ApplicationScoped; | ||
| import jakarta.enterprise.inject.Alternative; | ||
| import jakarta.annotation.Priority; | ||
| import jakarta.inject.Inject; | ||
|
|
||
| @ApplicationScoped | ||
| @Alternative | ||
| @Priority(100) // Higher than MicroProfileConfigProvider's priority of 50 | ||
| public class SpringConfigProvider implements A2AConfigProvider { | ||
|
|
||
| @Inject | ||
| Environment env; | ||
|
|
||
| @Inject | ||
| DefaultValuesConfigProvider defaultValues; | ||
|
|
||
| @Override | ||
| public String getValue(String name) { | ||
| String value = env.getProperty(name); | ||
| if (value != null) { | ||
| return value; | ||
| } | ||
| // Fallback to defaults | ||
| return defaultValues.getValue(name); | ||
| } | ||
|
|
||
| @Override | ||
| public Optional<String> getOptionalValue(String name) { | ||
| String value = env.getProperty(name); | ||
| if (value != null) { | ||
| return Optional.of(value); | ||
| } | ||
| return defaultValues.getOptionalValue(name); | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
The example provided for a custom SpringConfigProvider is a bit confusing as it mixes Jakarta EE CDI annotations (@ApplicationScoped, @Alternative, @Inject) with a Spring Framework concept (Environment). A typical Spring application would use Spring's own annotations (@Component, @Autowired, etc.) and may not have a CDI container running. This could be misleading for developers trying to integrate with Spring.
To make the example more idiomatic for Spring users, consider replacing it with one that uses Spring annotations and explains how to wire up the beans in a Spring context. For instance, DefaultValuesConfigProvider would need to be instantiated and initialized manually if not in a CDI environment.
Here's a more idiomatic Spring example you could adapt:
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.core.env.Environment;
import org.springframework.stereotype.Component;
import org.springframework.context.annotation.Primary;
// other imports...
@Component
@Primary // To ensure this provider is used over others
public class SpringConfigProvider implements A2AConfigProvider {
private final Environment env;
private final DefaultValuesConfigProvider defaultValues;
@Autowired
public SpringConfigProvider(Environment env) {
this.env = env;
// Since we are outside a CDI environment, we manage DefaultValuesConfigProvider manually.
this.defaultValues = new DefaultValuesConfigProvider();
this.defaultValues.init(); // This call is needed to load the default properties.
}
@Override
public String getValue(String name) {
String value = env.getProperty(name);
if (value != null) {
return value;
}
return defaultValues.getValue(name);
}
@Override
public Optional<String> getOptionalValue(String name) {
return Optional.ofNullable(env.getProperty(name))
.or(() -> defaultValues.getOptionalValue(name));
}
}There was a problem hiding this comment.
Rather than going all out Spring (which I know nothing about), I renamed the class so it no longer looks like Spring
...le-config/src/test/java/io/a2a/integrations/microprofile/MicroProfileConfigProviderTest.java
Show resolved
Hide resolved
integrations/microprofile-config/src/test/resources/application.properties
Show resolved
Hide resolved
a2aproject#468) Default values will now be used by default. You can supply your own by providing a CDI bean with a higher priority. Also, there is a new 2a-java-sdk-microprofile-config with the previous MicroProfile Config capabilities. If used, this will allow MicroProfile Config configurations of the properties. The reference implementations use this new module Fixes a2aproject#467 🦕
#469) Default values will now be used by default. You can supply your own by providing a CDI bean with a higher priority. Also, there is a new a2a-java-sdk-microprofile-config with the previous MicroProfile Config capabilities. If used, this will allow MicroProfile Config configurations of the properties. The reference implementations use this new module Fixes #467 🦕 Upstream: #468
a2aproject#468) Default values will now be used by default. You can supply your own by providing a CDI bean with a higher priority. Also, there is a new 2a-java-sdk-microprofile-config with the previous MicroProfile Config capabilities. If used, this will allow MicroProfile Config configurations of the properties. The reference implementations use this new module Fixes a2aproject#467 🦕
This came up in a Gemini review on the backport to 0.3.x This is a follow up for PR a2aproject#468 and issue a2aproject#467
Default values will now be used by default. You can supply your own by providing a CDI bean with a higher priority.
Also, there is a new 2a-java-sdk-microprofile-config with the previous MicroProfile Config capabilities. If used, this will allow MicroProfile Config configurations of the properties. The reference implementations use this new module
Fixes #467 🦕