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
[SUREFIRE-1954] move inner class ProviderList to upper level #393
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
package org.apache.maven.plugin.surefire; | ||
package org.apache.maven.surefire.providerapi; | ||
|
||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one | ||
|
@@ -22,7 +22,7 @@ | |
/** | ||
* @author Kristian Rosenvold | ||
*/ | ||
interface ConfigurableProviderInfo | ||
public interface ConfigurableProviderInfo | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @slawekjaranowski This interface is only used ASM class or you think that other project which extends the ASM class can instantiate it in the subclass of ASM class? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. before we have, in package:
Now are moved to |
||
extends ProviderInfo | ||
{ | ||
ProviderInfo instantiate( String providerName ); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
package org.apache.maven.surefire.providerapi; | ||
|
||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one | ||
* or more contributor license agreements. See the NOTICE file | ||
* distributed with this work for additional information | ||
* regarding copyright ownership. The ASF licenses this file | ||
* to you under the Apache License, Version 2.0 (the | ||
* "License"); you may not use this file except in compliance | ||
* with the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
import javax.annotation.Nonnull; | ||
|
||
import java.io.IOException; | ||
import java.util.ArrayList; | ||
import java.util.List; | ||
import java.util.Set; | ||
|
||
import org.apache.maven.surefire.api.provider.SurefireProvider; | ||
import org.codehaus.plexus.component.annotations.Component; | ||
import org.codehaus.plexus.component.annotations.Requirement; | ||
import org.codehaus.plexus.logging.Logger; | ||
|
||
import static java.lang.Thread.currentThread; | ||
|
||
/** | ||
* @author Kristian Rosenvold | ||
*/ | ||
@Component( role = ProviderDetector.class ) | ||
public final class ProviderDetector | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nicely extracted class. I want to ask about the modifiers There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. used in ASM from another package ... |
||
{ | ||
@Requirement | ||
private Logger logger; | ||
|
||
@Requirement | ||
private ServiceLoader serviceLoader; | ||
|
||
@Nonnull | ||
public List<ProviderInfo> resolve( ConfigurableProviderInfo dynamicProvider, ProviderInfo... wellKnownProviders ) | ||
{ | ||
List<ProviderInfo> providersToRun = new ArrayList<>(); | ||
Set<String> manuallyConfiguredProviders = getManuallyConfiguredProviders(); | ||
for ( String name : manuallyConfiguredProviders ) | ||
{ | ||
ProviderInfo wellKnown = findByName( name, wellKnownProviders ); | ||
ProviderInfo providerToAdd = wellKnown != null ? wellKnown : dynamicProvider.instantiate( name ); | ||
logger.info( "Using configured provider " + providerToAdd.getProviderName() ); | ||
providersToRun.add( providerToAdd ); | ||
} | ||
return manuallyConfiguredProviders.isEmpty() ? autoDetectOneWellKnownProvider( wellKnownProviders ) | ||
: providersToRun; | ||
} | ||
|
||
@Nonnull | ||
private List<ProviderInfo> autoDetectOneWellKnownProvider( ProviderInfo... wellKnownProviders ) | ||
{ | ||
List<ProviderInfo> providersToRun = new ArrayList<>(); | ||
for ( ProviderInfo wellKnownProvider : wellKnownProviders ) | ||
{ | ||
if ( wellKnownProvider.isApplicable() ) | ||
{ | ||
logger.info( "Using auto detected provider " + wellKnownProvider.getProviderName() ); | ||
providersToRun.add( wellKnownProvider ); | ||
return providersToRun; | ||
} | ||
} | ||
return providersToRun; | ||
} | ||
|
||
private Set<String> getManuallyConfiguredProviders() | ||
{ | ||
try | ||
{ | ||
ClassLoader cl = currentThread().getContextClassLoader(); | ||
return serviceLoader.lookup( SurefireProvider.class, cl ); | ||
} | ||
catch ( IOException e ) | ||
{ | ||
throw new RuntimeException( e ); | ||
} | ||
} | ||
|
||
private ProviderInfo findByName( String providerClassName, ProviderInfo... wellKnownProviders ) | ||
{ | ||
for ( ProviderInfo wellKnownProvider : wellKnownProviders ) | ||
{ | ||
if ( wellKnownProvider.getProviderName().equals( providerClassName ) ) | ||
{ | ||
return wellKnownProvider; | ||
} | ||
} | ||
return null; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ | |
*/ | ||
|
||
import javax.annotation.Nonnull; | ||
|
||
import java.io.BufferedReader; | ||
import java.io.IOException; | ||
import java.io.InputStream; | ||
|
@@ -29,6 +30,8 @@ | |
import java.util.HashSet; | ||
import java.util.Set; | ||
|
||
import org.codehaus.plexus.component.annotations.Component; | ||
|
||
import static java.lang.Character.isJavaIdentifierPart; | ||
import static java.lang.Character.isJavaIdentifierStart; | ||
import static java.util.Collections.emptySet; | ||
|
@@ -42,7 +45,8 @@ | |
* | ||
* @since 2.20 | ||
*/ | ||
public final class ServiceLoader | ||
@Component( role = ServiceLoader.class ) | ||
public class ServiceLoader | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @slawekjaranowski Can this Component be also There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I use mock There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah ok, so let it be, the tests with mock are more important |
||
{ | ||
|
||
@Nonnull | ||
|
@@ -67,7 +71,7 @@ public <T> Set<T> load( Class<T> clazz, ClassLoader classLoader ) | |
|
||
@Nonnull | ||
public Set<String> lookup( Class<?> clazz, ClassLoader classLoader ) | ||
throws IOException | ||
throws IOException | ||
{ | ||
final String resourceName = "META-INF/services/" + clazz.getName(); | ||
|
||
|
@@ -90,7 +94,7 @@ public Set<String> lookup( Class<?> clazz, ClassLoader classLoader ) | |
@Nonnull | ||
@SuppressWarnings( "checkstyle:innerassignment" ) | ||
private static Set<String> lookupSpiImplementations( final Enumeration<URL> urlEnumeration ) | ||
throws IOException | ||
throws IOException | ||
{ | ||
final Set<String> names = new HashSet<>(); | ||
nextUrl: | ||
|
@@ -143,7 +147,7 @@ private static Set<String> lookupSpiImplementations( final Enumeration<URL> urlE | |
|
||
@Nonnull | ||
private static BufferedReader getReader( @Nonnull URL url ) | ||
throws IOException | ||
throws IOException | ||
{ | ||
final InputStream inputStream = url.openStream(); | ||
final InputStreamReader inputStreamReader = new InputStreamReader( inputStream ); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slawekjaranowski Why the package has changed? There are probably many classes in the new package, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put all classes responsible for providers detecting in one package.
Before we have:
org.apache.maven.plugin.surefire.booterclient.ProviderDetector
which use
org.apache.maven.surefire.providerapi.ServiceLoader
and
ProviderDetector
was used only byAbstractSurefireMojo
so one responsibility was in many package.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I see there are now 5 classes. LGTM