Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pico lookup method returning <T> from generic criteria #6582

Merged
merged 1 commit into from
Apr 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 8 additions & 15 deletions pico/api/src/main/java/io/helidon/pico/api/Services.java
Original file line number Diff line number Diff line change
Expand Up @@ -110,26 +110,23 @@ <T> Optional<ServiceProvider<T>> lookupFirst(Class<T> type,
* Retrieves the first match based upon the passed service info criteria.
*
* @param criteria the criteria to find
* @param <T> the type of the service
* @return the best service provider
* @throws io.helidon.pico.api.PicoException if resolution fails to resolve a match
*/
@SuppressWarnings("unchecked")
default <T> ServiceProvider<T> lookup(ServiceInfoCriteria criteria) {
return (ServiceProvider<T>) lookupFirst(criteria, true).orElseThrow();
default ServiceProvider<?> lookup(ServiceInfoCriteria criteria) {
return lookupFirst(criteria, true).orElseThrow();
}

/**
* Retrieves the first match based upon the passed service info criteria.
*
* @param criteria the criteria to find
* @param expected indicates whether the provider should throw if a match is not found
* @param <T> the type of the service
* @return the best service provider matching the criteria, or {@code empty} if (@code expected = false) and no match found
* @throws io.helidon.pico.api.PicoException if expected=true and resolution fails to resolve a match
*/
<T> Optional<ServiceProvider<T>> lookupFirst(ServiceInfoCriteria criteria,
boolean expected);
Optional<ServiceProvider<?>> lookupFirst(ServiceInfoCriteria criteria,
boolean expected);

/**
* Retrieves the first match based upon the passed service info criteria.
Expand All @@ -140,13 +137,11 @@ <T> Optional<ServiceProvider<T>> lookupFirst(ServiceInfoCriteria criteria,
* </pre>
*
* @param criteria the criteria to find
* @param <T> the type of the service
* @return the best service provider matching the criteria
* @throws io.helidon.pico.api.PicoException if resolution fails to resolve a match
*/
@SuppressWarnings("unchecked")
default <T> ServiceProvider<T> lookupFirst(ServiceInfoCriteria criteria) {
return (ServiceProvider<T>) lookupFirst(criteria, true).orElseThrow();
default ServiceProvider<?> lookupFirst(ServiceInfoCriteria criteria) {
return lookupFirst(criteria, true).orElseThrow();
}

/**
Expand Down Expand Up @@ -179,12 +174,10 @@ default <T> ServiceProvider<T> lookupFirst(Class<T> type) {
* Retrieve all services that match the criteria.
*
* @param criteria the criteria to find
* @param <T> the type of the service
* @return the list of service providers matching criteria
*/
@SuppressWarnings({"unchecked", "rawtypes"})
default <T> List<ServiceProvider<T>> lookupAll(ServiceInfoCriteria criteria) {
return (List) lookupAll(criteria, false);
default List<ServiceProvider<?>> lookupAll(ServiceInfoCriteria criteria) {
return lookupAll(criteria, false);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public void startupAndShutdownRunLevelServices() {
ServiceInfoCriteria criteria = DefaultServiceInfoCriteria.builder()
.runLevel(RunLevel.STARTUP)
.build();
List<ServiceProvider<Object>> startups = services.lookupAll(criteria);
List<ServiceProvider<?>> startups = services.lookupAll(criteria);
List<String> desc = startups.stream().map(ServiceProvider::description).collect(Collectors.toList());
assertThat(desc,
contains(ASimpleRunLevelService.class.getSimpleName() + ":INIT"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ void testRegistry() {
DefaultServiceInfoCriteria criteria = DefaultServiceInfoCriteria.builder()
.addQualifier(DefaultQualifierAndValue.create(ConfiguredBy.class))
.build();
List<ServiceProvider<Object>> list = services.lookupAll(criteria);
List<ServiceProvider<?>> list = services.lookupAll(criteria);
List<String> desc = list.stream().map(ServiceProvider::description).collect(Collectors.toList());
assertThat("root providers are config-driven, auto-started services unless overridden to not be driven", desc,
contains("ASingletonService{root}:ACTIVE",
Expand Down Expand Up @@ -159,7 +159,7 @@ void testRegistry() {
assertThat("root providers expected here since no configuration for this service", desc,
contains("FakeTlsWSNotDrivenByCB{root}:PENDING"));

ServiceProvider<Object> fakeTlsProvider = list.get(0);
ServiceProvider<?> fakeTlsProvider = list.get(0);
PicoServiceProviderException e = assertThrows(PicoServiceProviderException.class, fakeTlsProvider::get);
assertThat("There is no configuration, so cannot activate this service", e.getMessage(),
equalTo("Expected to find a match: service provider: FakeTlsWSNotDrivenByCB{root}:PENDING"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ protected void innerExecute() {
// get the application creator only after pico services were initialized (we need to ignore any existing apps)
ApplicationCreator creator = applicationCreator();

List<ServiceProvider<Module>> allModules = services
List<ServiceProvider<?>> allModules = services
.lookupAll(DefaultServiceInfoCriteria.builder().addContractImplemented(Module.class.getName()).build());
getLog().info("processing modules: " + toDescriptions(allModules));
if (allModules.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,46 +216,48 @@ public boolean reset(boolean deep) {
}

@Override
@SuppressWarnings({"unchecked", "rawtypes"})
public <T> Optional<ServiceProvider<T>> lookupFirst(Class<T> type,
boolean expected) {
DefaultServiceInfoCriteria criteria = DefaultServiceInfoCriteria.builder()
.addContractImplemented(type.getName())
.build();
return lookupFirst(criteria, expected);
return (Optional) lookupFirst(criteria, expected);
tomas-langer marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
@SuppressWarnings({"unchecked", "rawtypes"})
public <T> Optional<ServiceProvider<T>> lookupFirst(Class<T> type,
String name,
boolean expected) {
DefaultServiceInfoCriteria criteria = DefaultServiceInfoCriteria.builder()
.addContractImplemented(type.getName())
.addQualifier(DefaultQualifierAndValue.createNamed(name))
.build();
return lookupFirst(criteria, expected);
return (Optional) lookupFirst(criteria, expected);
}

@Override
public <T> Optional<ServiceProvider<T>> lookupFirst(ServiceInfoCriteria criteria,
boolean expected) {
List<ServiceProvider<T>> result = lookup(criteria, expected, 1);
public Optional<ServiceProvider<?>> lookupFirst(ServiceInfoCriteria criteria,
boolean expected) {
List<ServiceProvider<?>> result = lookup(criteria, expected, 1);
assert (!expected || !result.isEmpty());
return (result.isEmpty()) ? Optional.empty() : Optional.of(result.get(0));
}

@Override
@SuppressWarnings({"unchecked", "rawtypes"})
public <T> List<ServiceProvider<T>> lookupAll(Class<T> type) {
DefaultServiceInfoCriteria serviceInfo = DefaultServiceInfoCriteria.builder()
.addContractImplemented(type.getName())
.build();
return lookup(serviceInfo, false, Integer.MAX_VALUE);
return (List) lookup(serviceInfo, false, Integer.MAX_VALUE);
}

@Override
@SuppressWarnings({"rawtypes", "unchecked"})
public List<ServiceProvider<?>> lookupAll(ServiceInfoCriteria criteria,
boolean expected) {
List<ServiceProvider<?>> result = (List) lookup(criteria, expected, Integer.MAX_VALUE);
List<ServiceProvider<?>> result = lookup(criteria, expected, Integer.MAX_VALUE);
assert (!expected || !result.isEmpty());
return result;
}
Expand Down Expand Up @@ -346,10 +348,10 @@ Metrics metrics() {
.build();
}

@SuppressWarnings({"unchecked", "rawtypes"})
<T> List<ServiceProvider<T>> lookup(ServiceInfoCriteria criteria,
boolean expected,
int limit) {
@SuppressWarnings({"rawtypes"})
List<ServiceProvider<?>> lookup(ServiceInfoCriteria criteria,
boolean expected,
int limit) {
List<ServiceProvider<?>> result;

lookupCount.incrementAndGet();
Expand Down Expand Up @@ -389,7 +391,7 @@ <T> List<ServiceProvider<T>> lookup(ServiceInfoCriteria criteria,
cacheLookupCount.incrementAndGet();
if (result != null) {
cacheHitCount.incrementAndGet();
return (List) result;
return result;
}
}

Expand All @@ -411,7 +413,7 @@ <T> List<ServiceProvider<T>> lookup(ServiceInfoCriteria criteria,
cache.put(criteria, List.copyOf(result));
}

return (List) result;
return result;
}

ServiceProvider<?> serviceProviderFor(String serviceTypeName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ void runtimeWithNoInterception() throws Exception {
.addContractImplemented(Closeable.class.getName())
.includeIntercepted(true)
.build();
List<ServiceProvider<Closeable>> closeableProviders = services.lookupAll(criteria);
List<ServiceProvider<?>> closeableProviders = services.lookupAll(criteria);
assertThat("the interceptors should always be weighted higher than the non-interceptors",
toDescriptions(closeableProviders),
contains("XImpl$$Pico$$Interceptor:INIT", "YImpl$$Pico$$Interceptor:INIT",
Expand Down Expand Up @@ -160,7 +160,7 @@ void runtimeWithNoInterception() throws Exception {
equalTo("forced"));

// we cannot look up by service type here - we need to instead lookup by one of the interfaces
ServiceProvider<Closeable> yimplProvider = services
ServiceProvider<?> yimplProvider = services
.lookupFirst(
DefaultServiceInfoCriteria.builder()
.addContractImplemented(Closeable.class.getName())
Expand Down Expand Up @@ -242,7 +242,7 @@ void runtimeWithInterception() throws Exception {
equalTo(1));

// we cannot look up by service type here - we need to instead lookup by one of the interfaces
ServiceProvider<Closeable> yimplProvider = services
ServiceProvider<?> yimplProvider = services
.lookupFirst(
DefaultServiceInfoCriteria.builder()
.addContractImplemented(Closeable.class.getName())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ void startupAndShutdownCallsPostConstructAndPreDestroy() {

@Test
void knownProviders() {
List<ServiceProvider<Provider>> providers = services.lookupAll(
List<ServiceProvider<?>> providers = services.lookupAll(
DefaultServiceInfoCriteria.builder().addContractImplemented(Provider.class.getName()).build());
List<String> desc = providers.stream().map(ServiceProvider::description).collect(Collectors.toList());
// note that order matters here
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ void codegenHelloWorldApplication() {

PicoServices picoServices = PicoServices.picoServices().orElseThrow();
Services services = picoServices.services();
List<ServiceProvider<Object>> serviceProviders = services.lookupAll(allServices);
List<ServiceProvider<?>> serviceProviders = services.lookupAll(allServices);

List<TypeName> serviceTypeNames = serviceProviders.stream()
.map(sp -> DefaultTypeName.createFromTypeName(sp.serviceInfo().serviceTypeName()))
Expand Down