Skip to content

Commit

Permalink
improve coverage of metadata validation
Browse files Browse the repository at this point in the history 
call validators when dealing with metadata
  • Loading branch information
@kwin
kwin committed Jan 3, 2022
1 parent 2a33eff commit f8181f0
Show file tree
Hide file tree
Showing 10 changed files with 381 additions and 73 deletions.
12 changes: 10 additions & 2 deletions ...re/src/main/java/org/apache/maven/plugin/prefix/internal/DefaultPluginPrefixResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@

import org.apache.maven.artifact.repository.metadata.Metadata;
import org.apache.maven.artifact.repository.metadata.io.MetadataReader;
import org.apache.maven.artifact.repository.metadata.validator.MetadataValidator;
import org.apache.maven.artifact.repository.metadata.validator.MetadataValidator.Level;
import org.apache.maven.model.Build;
import org.apache.maven.model.Plugin;
import org.apache.maven.plugin.BuildPluginManager;
Expand All @@ -39,6 +41,7 @@
import org.apache.maven.plugin.prefix.PluginPrefixRequest;
import org.apache.maven.plugin.prefix.PluginPrefixResolver;
import org.apache.maven.plugin.prefix.PluginPrefixResult;
import org.apache.maven.repository.internal.RepositoryListenerMetadataProblemCollector;
import org.eclipse.aether.DefaultRepositorySystemSession;
import org.eclipse.aether.RepositoryEvent;
import org.eclipse.aether.RepositoryEvent.EventType;
Expand Down Expand Up @@ -72,16 +75,19 @@ public class DefaultPluginPrefixResolver
private final BuildPluginManager pluginManager;
private final RepositorySystem repositorySystem;
private final MetadataReader metadataReader;
private final MetadataValidator metadataValidator;

@Inject
public DefaultPluginPrefixResolver(
BuildPluginManager pluginManager,
RepositorySystem repositorySystem,
MetadataReader metadataReader )
MetadataReader metadataReader,
MetadataValidator metadataValidator )
{
this.pluginManager = pluginManager;
this.repositorySystem = repositorySystem;
this.metadataReader = metadataReader;
this.metadataValidator = metadataValidator;
}

public PluginPrefixResult resolve( PluginPrefixRequest request )
Expand Down Expand Up @@ -259,7 +265,9 @@ private PluginPrefixResult resolveFromRepository( PluginPrefixRequest request, R
Map<String, ?> options = Collections.singletonMap( MetadataReader.IS_STRICT, Boolean.FALSE );

Metadata pluginGroupMetadata = metadataReader.read( metadata.getFile(), options );

metadataValidator.validate( pluginGroupMetadata, Level.GROUP_ID, null,
new RepositoryListenerMetadataProblemCollector( request.getRepositorySession(), repository,
trace, metadata ) );
List<org.apache.maven.artifact.repository.metadata.Plugin> plugins = pluginGroupMetadata.getPlugins();

if ( plugins != null )
Expand Down
10 changes: 9 additions & 1 deletion .../src/main/java/org/apache/maven/plugin/version/internal/DefaultPluginVersionResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
import org.apache.maven.artifact.repository.metadata.Metadata;
import org.apache.maven.artifact.repository.metadata.Versioning;
import org.apache.maven.artifact.repository.metadata.io.MetadataReader;
import org.apache.maven.artifact.repository.metadata.validator.MetadataValidator;
import org.apache.maven.artifact.repository.metadata.validator.MetadataValidator.Level;
import org.apache.maven.model.Build;
import org.apache.maven.model.Plugin;
import org.apache.maven.plugin.MavenPluginManager;
Expand All @@ -46,6 +48,7 @@
import org.apache.maven.plugin.version.PluginVersionResolutionException;
import org.apache.maven.plugin.version.PluginVersionResolver;
import org.apache.maven.plugin.version.PluginVersionResult;
import org.apache.maven.repository.internal.RepositoryListenerMetadataProblemCollector;
import org.codehaus.plexus.util.StringUtils;
import org.eclipse.aether.RepositoryEvent;
import org.eclipse.aether.RepositoryEvent.EventType;
Expand Down Expand Up @@ -83,18 +86,22 @@ public class DefaultPluginVersionResolver
private final Logger logger = LoggerFactory.getLogger( getClass() );
private final RepositorySystem repositorySystem;
private final MetadataReader metadataReader;
private final MetadataValidator metadataValidator;
private final MavenPluginManager pluginManager;
private final VersionScheme versionScheme;


@Inject
public DefaultPluginVersionResolver(
RepositorySystem repositorySystem,
MetadataReader metadataReader,
MetadataValidator metadataValidator,
MavenPluginManager pluginManager,
VersionScheme versionScheme )
{
this.repositorySystem = repositorySystem;
this.metadataReader = metadataReader;
this.metadataValidator = metadataValidator;
this.pluginManager = pluginManager;
this.versionScheme = versionScheme;
}
Expand Down Expand Up @@ -314,7 +321,8 @@ private void mergeMetadata( RepositorySystemSession session, RequestTrace trace,
Map<String, ?> options = Collections.singletonMap( MetadataReader.IS_STRICT, Boolean.FALSE );

Metadata repoMetadata = metadataReader.read( metadata.getFile(), options );

metadataValidator.validate( repoMetadata, Level.GROUP_ID, null,
new RepositoryListenerMetadataProblemCollector( session, repository, trace, metadata ) );
mergeMetadata( versions, repoMetadata, repository );
}
catch ( IOException e )
Expand Down
8 changes: 8 additions & 0 deletions maven-repository-metadata/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ under the License.
<description>Per-directory local and remote repository metadata.</description>

<dependencies>
<dependency>
<groupId>javax.inject</groupId>
<artifactId>javax.inject</artifactId>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-utils</artifactId>
Expand All @@ -52,6 +56,10 @@ under the License.
</models>
</configuration>
</plugin>
<plugin>
<groupId>org.eclipse.sisu</groupId>
<artifactId>sisu-maven-plugin</artifactId>
</plugin>
</plugins>
</build>
</project>
155 changes: 118 additions & 37 deletions ...ava/org/apache/maven/artifact/repository/metadata/validator/DefaultMetadataValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,51 +21,109 @@

import java.util.Collection;

import javax.inject.Named;
import javax.inject.Singleton;

import org.apache.maven.artifact.repository.metadata.Metadata;
import org.apache.maven.artifact.repository.metadata.Plugin;
import org.apache.maven.artifact.repository.metadata.SnapshotVersion;
import org.apache.maven.artifact.repository.metadata.validator.MetadataProblemCollector.Severity;

/**
* https://maven.apache.org/ref/LATEST/maven-repository-metadata/
*
* Validates repository metadata on different levels.
*
* @see <a href="https://maven.apache.org/ref/current/maven-repository-metadata/index.html">Repository Metadata Model</a>
* @see <a href="https://maven.apache.org/ref/current/maven-repository-metadata/repository-metadata.html">Repository Metadata Descriptor</a>
*/
@Named
@Singleton
public class DefaultMetadataValidator implements MetadataValidator
{

@Override
public void validate( Metadata metadata, MetadataProblemCollector problems )
{
validate( metadata, !metadata.getPlugins().isEmpty(), problems );
// some heuristic to determine level
final Level level;
if ( !metadata.getPlugins().isEmpty() )
{
level = Level.GROUP_ID;
}
else if ( metadata.getVersioning() != null && !metadata.getVersioning().getSnapshotVersions().isEmpty() )
{
level = Level.VERSION;
}
else
{
level = Level.ARTIFACT_ID;
}
validate( metadata, level, null, problems );
}

@Override
public void validate( Metadata metadata, boolean isGroupMetadata, MetadataProblemCollector problems )
public void validate( Metadata metadata, Level level, Boolean isSnapshot, MetadataProblemCollector problems )
{
// group metadata is only supposed to contain plugins
if ( isGroupMetadata )
{
for ( Plugin plugin : metadata.getPlugins() )
{
validateStringNotEmpty ( problems, "plugins.plugin.name", plugin.getName(),
plugin.getArtifactId() );
validateStringNotEmpty ( problems, "plugins.plugin.prefix", plugin.getPrefix(),
plugin.getName() );
validateStringNotEmpty ( problems, "plugins.plugin.artifactId", plugin.getArtifactId(),
plugin.getName() );
}
validateNullOrEmptyCollection ( problems, "groupId", metadata.getGroupId(), null );
validateNullOrEmptyCollection ( problems, "artifactId", metadata.getArtifactId(), null );
validateNullOrEmptyCollection ( problems, "version", metadata.getVersion(), null );
validateNullOrEmptyCollection ( problems, "versioning", metadata.getVersioning(), null );
}
else
switch ( level )
{
validateStringNotEmpty ( problems, "groupId", metadata.getGroupId(), null );
validateStringNotEmpty ( problems, "artifactId", metadata.getArtifactId(), null );
// validateStringNotEmpty ( problems, "version", metadata.getVersion(), null ); // only for snapshots
// TODO:, distringuish between artifactId level and version level and snapshot/release
// according to MDO nothing is required, but at least latest and one or more versions are reasonable
// to assume
case GROUP_ID:
for ( Plugin plugin : metadata.getPlugins() )
{
validateStringNotEmpty ( problems, "plugins.plugin.name", plugin.getName(),
plugin.getArtifactId(), level );
validateStringNotEmpty ( problems, "plugins.plugin.prefix", plugin.getPrefix(),
plugin.getName(), level );
validateStringNotEmpty ( problems, "plugins.plugin.artifactId", plugin.getArtifactId(),
plugin.getName(), level );
}
validateNullOrEmptyCollection ( problems, "groupId", metadata.getGroupId(), null, level );
validateNullOrEmptyCollection ( problems, "artifactId", metadata.getArtifactId(), null, level );
validateNullOrEmptyCollection ( problems, "version", metadata.getVersion(), null, level );
validateNullOrEmptyCollection ( problems, "versioning", metadata.getVersioning(), null, level );
break;
case ARTIFACT_ID:
validateStringNotEmpty ( problems, "groupId", metadata.getGroupId(), null, level );
validateStringNotEmpty ( problems, "artifactId", metadata.getArtifactId(), null, level );
validateNullOrEmptyCollection ( problems, "version", metadata.getVersion(), null, level );
if ( validateNotNull( problems, "versioning", metadata.getVersioning(), null, level ) )
{
validateNotEmptyCollection( problems, "versioning.versions",
metadata.getVersioning().getVersions(), null, level );
validateNullOrEmptyCollection( problems, "versioning.snapshotVersions",
metadata.getVersioning().getSnapshotVersions(), null, level );
}
validateNullOrEmptyCollection ( problems, "plugins", metadata.getPlugins(), null, level );
// TODO: release or latest is mandatory?
break;
default:
if ( isSnapshot == Boolean.FALSE )
{
validateNullOrEmptyCollection ( problems, "groupId", metadata.getGroupId(), null, level );
validateNullOrEmptyCollection ( problems, "artifactId", metadata.getArtifactId(), null, level );
validateNullOrEmptyCollection ( problems, "version", metadata.getVersion(), null, level );
validateNullOrEmptyCollection ( problems, "versioning", metadata.getVersioning(), null, level );

}
else
{
validateStringNotEmpty ( problems, "groupId", metadata.getGroupId(), null, level );
validateStringNotEmpty ( problems, "artifactId", metadata.getArtifactId(), null, level );
validateStringNotEmpty ( problems, "version", metadata.getArtifactId(), null, level );
if ( validateNotNull( problems, "versioning", metadata.getVersioning(), null, level ) )
{
for ( SnapshotVersion version : metadata.getVersioning().getSnapshotVersions() )
{
validateStringNotEmpty ( problems, "versioning.snapshotVersions.snapshotVersion.extension",
version.getExtension(), version.getVersion(), level );
validateStringNotEmpty ( problems, "versioning.snapshotVersions.snapshotVersion.value",
version.getVersion(), null, level );
// TODO validate updated timestamp?
}
validateNotNull( problems, "versioning.snapshotVersions",
metadata.getVersioning().getSnapshotVersions(), null, level );
}
}
validateNullOrEmptyCollection ( problems, "plugins", metadata.getPlugins(), null, level );
break;
}
}

Expand All @@ -82,9 +140,9 @@ public void validate( Metadata metadata, boolean isGroupMetadata, MetadataProble
* </ul>
*/
private static boolean validateStringNotEmpty( MetadataProblemCollector problems, String fieldName, String string,
String sourceHint )
String sourceHint, Level level )
{
if ( !validateNotNull( problems, fieldName, string, sourceHint ) )
if ( !validateNotNull( problems, fieldName, string, sourceHint, level ) )
{
return false;
}
Expand All @@ -94,7 +152,7 @@ private static boolean validateStringNotEmpty( MetadataProblemCollector problems
return true;
}

addViolation( problems, Severity.ERROR, fieldName, sourceHint, "is missing" );
addViolation( problems, Severity.ERROR, fieldName, sourceHint, level, "is missing" );

return false;
}
Expand All @@ -107,14 +165,14 @@ private static boolean validateStringNotEmpty( MetadataProblemCollector problems
* </ul>
*/
private static boolean validateNotNull( MetadataProblemCollector problems, String fieldName, Object object,
String sourceHint )
String sourceHint, Level level )
{
if ( object != null )
{
return true;
}

addViolation( problems, Severity.ERROR, fieldName, sourceHint, "is missing" );
addViolation( problems, Severity.ERROR, fieldName, sourceHint, level, "is missing" );

return false;
}
Expand All @@ -127,20 +185,40 @@ private static boolean validateNotNull( MetadataProblemCollector problems, Strin
* </ul>
*/
private static boolean validateNullOrEmptyCollection( MetadataProblemCollector problems, String fieldName,
Object object, String sourceHint )
Object object, String sourceHint, Level level )
{
if ( object == null || ( object instanceof Collection && Collection.class.cast( object ).isEmpty() ) )
{
return true;
}

addViolation( problems, Severity.WARNING, fieldName, sourceHint, "is unused" );
addViolation( problems, Severity.WARNING, fieldName, sourceHint, level, "is unused" );

return false;
}

/**
* Asserts:
* <p/>
* <ul>
* <li><code>string != null</code>
* </ul>
*/
private static boolean validateNotEmptyCollection( MetadataProblemCollector problems, String fieldName,
Collection<?> collection, String sourceHint, Level level )
{
if ( !collection.isEmpty() )
{
return true;
}

addViolation( problems, Severity.WARNING, fieldName, sourceHint, level, "is empty collection" );

return false;
}

private static void addViolation( MetadataProblemCollector problems, Severity severity, String fieldName,
String sourceHint, String message )
String sourceHint, Level level, String message )
{
StringBuilder buffer = new StringBuilder( 256 );
buffer.append( '\'' ).append( fieldName ).append( '\'' );
Expand All @@ -149,7 +227,10 @@ private static void addViolation( MetadataProblemCollector problems, Severity se
{
buffer.append( " for " ).append( sourceHint );
}

if ( level != null )
{
buffer.append( " on repository metadata level " ).append( level );
}
buffer.append( ' ' ).append( message );

problems.add( severity, buffer.toString(), -1, -1, null );
Expand Down
Loading

0 comments on commit f8181f0

Please sign in to comment.